Page MenuHomePhabricator

Add status table to Diffusion Branch manage page
ClosedPublic

Authored by chad on Aug 10 2017, 5:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 19, 11:05 AM
Unknown Object (File)
Tue, Dec 17, 8:57 PM
Unknown Object (File)
Fri, Dec 13, 6:56 PM
Unknown Object (File)
Thu, Dec 12, 11:28 PM
Unknown Object (File)
Thu, Dec 12, 1:54 AM
Unknown Object (File)
Sun, Dec 8, 6:32 PM
Unknown Object (File)
Sat, Dec 7, 4:02 AM
Unknown Object (File)
Thu, Nov 28, 1:19 PM
Subscribers

Details

Summary

Fixes T12832. Adds a basic table (not paginated?) to view tracking and autoclose status.

Test Plan

Review a large repository (Krita) with setting various states of tracking and autoclose.

image.png (1×2 px, 394 KB)

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

large repository

Does this repository have 10,000+ branches?

I thought I put a comment on the diff message - I'm not sure how to paginate this.

  • 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.

This revision now requires changes to proceed.Aug 10 2017, 5:28 PM

$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.

chad edited edge metadata.
  • add pager, use correct methods, check import status
This revision is now accepted and ready to land.Aug 10 2017, 8:25 PM
This revision was automatically updated to reflect the committed changes.