Allow API callers to retrieve reviewer information via a new "reviewers" attachment.
Details
Details
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- review1
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 16333 Build 21719: Run Core Tests Build 21718: arc lint + arc unit
Event Timeline
Comment Actions
- Get rid of isCurrent which is kind of meaningless and confusing.
- Add an isBlocking to try to future-proof against that being separated out.
Comment Actions
| src/applications/differential/engineextension/DifferentialReviewersSearchEngineAttachment.php | ||
|---|---|---|
| 42 | I know you're stripping this away, but out of curiosity, what would this have meant / why is it too complicated? Does it mean whether they've taken any action on the review? Or whether they've taken the last action? | |
Comment Actions
The meaning of isCurrent was "is this reviewer relevant for state calculations against the current state of the revision", which is kind of involved.
For example, a revision with one user who has "Rejected" is normally in state "Needs Revision", until the next update, but not if the author does "Request Review". When they do, they internally void the outstanding rejects and make them noncurrent.
But current-ness depends on a lot of internal state stuff and on differential.sticky-accept and I think it's ultimately too wrapped up in state calculation to be of much use. At best, you could use it to render "Accepted Current Diff" vs "Accepted Older Diff" some of the time, but it doesn't always mean that and there's no real human-readable label for what it does mean.