Fixes T12832. Adds a basic table (not paginated?) to view tracking and autoclose status.
Details
- Reviewers
epriestley - Maniphest Tasks
- T12832: Add autoclose table to Diffusion Manage
- Commits
- rPa7124f8f7a11: Add status table to Diffusion Branch manage page
Review a large repository (Krita) with setting various states of tracking and autoclose.
Diff Detail
- Repository
- rP Phabricator
- Branch
- diffusion-autoclose-table (branched from master)
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 17987 Build 24155: Run Core Tests Build 24154: arc lint + arc unit
Event Timeline
- You can paginate by following the logic in DiffusionBranchTableController, i.e. use PHUIPagerView to manage an offset and limit, and pass offset and limit to branchquery.
- You should use $repository->shouldAutocloseBranch() and $repository->shouldTrackBranch() to test if a branch is autoclose and tracked, respectively. The logic as written is incorrect because these fields accept regular expressions, not just literal branch names (see "Branches" in Diffusion User Guide: Managing Repositories).
src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php | ||
---|---|---|
198–203 | This is unnecessary if all columns are visible. |
$repository->shouldAutocloseBranch() seems to not work if the repository is still importing. It will just say "off". I can get a reason, "auto/importing". Not sure what to show the user.
That's PhabricatorRepository::BECAUSE_REPOSITORY_IMPORTING which means "This is an autoclose branch, but we aren't autoclosing commits on it yet because the repository hasn't finished its initial first-time import.".
This rule addresses the case where installs may choose to (for example) import the phabricator/ repository locally, but forget to disable "Autoclose". If we autoclosed during initial import, their install would see a lot of commits appear with Fixes Txxx (our commits from the Phabricator upstream) and incorrectly close thousands of tasks -- e.g., when I wrote Fixes T123 in some commit I meant our T123 -- https://secure.phabricator.com/T123 -- but if phabricator/ is imported into another install that install will have a different local T123 -- https://phabricator.some-company.com/T123.
You could display "Yes" or "Yes, But Not Until The Import Finishes", or hide the whole table.