Details
Details
- Reviewers
amckinley - Maniphest Tasks
- T2543: Add a formal "Draft" / "Not Yet Ready for Review" state to Differential
- Commits
- rARCf1ba27ffedd5: Add "arc diff --draft"
Ran "arc diff --draft" when creating; ran "arc diff --draft" to try to update a revision and got a failure.
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
src/workflow/ArcanistDiffWorkflow.php | ||
---|---|---|
593–596 | I guess we don't do too much of this in general, and we shouldn't force people to upgrade unless they're actually going to be using --draft, but I wonder if we should ever build an generic versioned API instead of explicitly doing checks like this. |
Comment Actions
We had a versioned API at one point and moved away from it (there's still some code in ConduitConnectConduitAPIMethod). Some reasons include:
- Prototypes, extensions, configuration, and the ability to install/uninstall applications mean that two installs of Phabricator at version X may expose different capabilities via the API.
- Sometimes we'll add a feature for a third-party, and not use it in arc until some time later. With capability tests, this isn't a problem. With versioning, we'd need to bump the version on every change, or accept that arc will claim that some versions which have the capability (between when it was added, and when the version was bumped) don't have the capability, which is confusing to users.
- The implementation forced upgrades after checking versions ("you are out of date, upgrade"), and users hated it. An implementation wouldn't have to do this, but if we replace capability tests with version tests and don't force upgrades, we end up with the same code, just if ($version >= $version_with_draft) instead of if ($has_draft_capability), but both blocks are still inline somewhere in the actual code along the execution pathway for the feature.
- Often, we can navigate API changes transparently (e.g., file.upload uses modern stuff if available and falls back if not).