Changeset View
Standalone View
src/repository/state/ArcanistRepositoryLocalState.php
Show First 20 Lines • Show All 186 Lines • ▼ Show 20 Lines | abstract class ArcanistRepositoryLocalState | ||||
final public function getRestoreCommandsForDisplay() { | final public function getRestoreCommandsForDisplay() { | ||||
return $this->newRestoreCommandsForDisplay(); | return $this->newRestoreCommandsForDisplay(); | ||||
} | } | ||||
protected function canStashChanges() { | protected function canStashChanges() { | ||||
return false; | return false; | ||||
} | } | ||||
/** | |||||
* Stash uncommitted changes temporarily. Use {@method:restoreStash()} to | |||||
* bring these changes back. | |||||
* | |||||
* Note that on Git repositories the stash acts as a stack, so saving the | |||||
* stash must match appropriately to restoring the stash. | |||||
* | |||||
* @return wild On Git this returns true, on Mercurial this returns a name | |||||
* (string) which references the stash that was made. This name | |||||
* should later be passed to {@method:restoreStash()}. | |||||
*/ | |||||
protected function saveStash() { | protected function saveStash() { | ||||
throw new PhutilMethodNotImplementedException(); | throw new PhutilMethodNotImplementedException(); | ||||
} | } | ||||
/** | |||||
* Restores changes that were previously stashed by {@method:saveStash()}. | |||||
* | |||||
* @param wild On Git this parameter is unused, on Mercurial this should be | |||||
* the name (string) returned from {@method:saveStash()}. | |||||
*/ | |||||
epriestley: This is neither here nor there, but my imagined "correct" use of this API is to treat the… | |||||
cspeckmimAuthorUnsubmitted Done Inline ActionsI agree with $ref being opaque and will update the comments here. At the time I think was more keen to validate my own understanding by writing it out without second-guessing whether it made sense to put the implementation-level details at this abstract level. cspeckmim: I agree with `$ref` being opaque and will update the comments here. At the time I think was… | |||||
cspeckmimAuthorUnsubmitted Done Inline ActionsAh though one point about this is that it does impact how Git repositories interract with this API. Someone following this API currently might assume that the following is valid, however it turns out to work properly with Mercurial but not in Git. save -> ref1 save -> ref2 restore(ref1) restore(ref2) It might not cause an error but the behavior would be unexpected. I think that was my original intention behind the first note on saveStash(). I'll clean up some of the details about what specifically is being returned but I think it would be useful to retain some notice about Git's behavior at least until (or if) the implementation supports that. cspeckmim: Ah though one point about this is that it does impact how Git repositories interract with this… | |||||
cspeckmimAuthorUnsubmitted Done Inline ActionsI think I'll make a new diff which updates GitBinaryAnalyzer to check for this better stash usage, and if not uses an internal stack to ensure that we don't accidentally run a restore out of order cspeckmim: I think I'll make a new diff which updates `GitBinaryAnalyzer` to check for this better stash… | |||||
protected function restoreStash($ref) { | protected function restoreStash($ref) { | ||||
throw new PhutilMethodNotImplementedException(); | throw new PhutilMethodNotImplementedException(); | ||||
} | } | ||||
protected function discardStash($ref) { | protected function discardStash($ref) { | ||||
throw new PhutilMethodNotImplementedException(); | throw new PhutilMethodNotImplementedException(); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 53 Lines • Show Last 20 Lines |
This is neither here nor there, but my imagined "correct" use of this API is to treat the "$ref" as opaque and always return the "$ref" you were handed under every VCS. Git happens to ignore it today but may not in some future version of the API, i.e. this use of the API would be "incorrect" but is arguably somewhat-supported by the comments:
I think this is really a bigger issue ("who is the audience for comments?") and I don't feel strongly about this, but I suspect if I was convinced that comments were worth getting right and tried to develop a guide for them, I'd probably end up with guidance like "don't put implementation details in a doc comment far away from the implementation?" and maybe "instead, put them inline next to the implementation" perhaps with a side of "annotate them in such-and-such a way and the documentation generator will extract them".