batches: exclude archived, deleted changesets from completion percentage calculation#46831
Conversation
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff f0d44cf...36c2299.
|
Bundle size report 📦
Look at the Statoscope report for a full comparison between the commits 36c2299 and 90ebc94 or learn more. Open explanation
|
BolajiOlajide
left a comment
There was a problem hiding this comment.
Approving to unblock.
| args.closed + | ||
| args.deleted + | ||
| args.merged + | ||
| args.draft + | ||
| args.open + | ||
| args.archived + |
There was a problem hiding this comment.
| args.closed + | |
| args.deleted + | |
| args.merged + | |
| args.draft + | |
| args.open + | |
| args.archived + | |
| args.merged + | |
| args.draft + | |
| args.open + |
Should archived & deleted changesets still be part of the total here? I think it shouldn't.
There was a problem hiding this comment.
I believe they should be here, actually. This is supposed to mirror the data we'd get from the backend, where total is literally just SELECT COUNT(*) FROM CHANGESETS.
eseliger
left a comment
There was a problem hiding this comment.
actually approving to unblock 😛
| const { changesetsStats: stats, diffStat } = batchChange | ||
| const percentComplete = stats.total === 0 ? 0 : ((stats.closed + stats.merged + stats.deleted) / stats.total) * 100 | ||
| const isCompleted = stats.closed + stats.merged + stats.deleted === stats.total | ||
| const percentComplete = |
There was a problem hiding this comment.
I wonder if at this point this should move in the backend, seems fairly complex logic needs to be applied in the frontend to get that value :D
There was a problem hiding this comment.
You're probably right. 🙂 I'm too lazy to do that right now but I'll leave a note!
Closes https://github.com/sourcegraph/sourcegraph/issues/46830.
This was surfaced in Slack and felt like a reasonable change.
The completion percentage is now defined based on two values: the total "actionable changesets" as the denominator, which is defined as the total minus any archived and deleted changesets; and the total "completed changesets" as the numerator, which is defined as the sum of merged and closed changesets.
There's been confusion about how the completion percentage is calculated at times before, as well as confusion over what "archived" means and how that's factored in to this percentage. For this reason, I've added an info icon + tooltip and a short description to the top of the archived changesets list, in the style of several of our admin pages where the contents of the container are explained in brief just above it. This description includes a link to the docs, too. You can see these changes below:
cc @danielmarquespt
Open to wording suggestions. 🙂
Test plan
Updated storybooks to use control knobs to test how different changeset stats affect the percentage. As in the screenshot above, you can verify archived was not included in the batch change completion percentage.
App preview:
Check out the client app preview documentation to learn more.