[WIP] Output stack graph to PR bodies #264

oleg-codaio · 2024-04-26T06:14:08Z

When creating/updating/syncing stacks, also include the latest stack of changes in each pull request description. This essentially replicates the av stack tree visualization in PRs:

This is off by default and can be enabled via pullRequest.writeStack (for those not yet using a merge queue).

Work in progress

Update av stack sync to include the current stack in all open PR descriptions
Update av pr create to include the current stack
Fix bug where av pr create doesn't include itself until re-sync
Fix bug where av stack submit doesn't include the stack
Figure out how to keep merged PRs included in descendant stacks (until the whole stack tree is merged into trunk)
Enable setting branch prefix via config

…odule Move stack utils to separate module

…scription Support writing stack to PR description

Write out stack on sync

Exclude branches on other trees

aviator-app · 2024-04-26T06:14:11Z

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was closed without merging. If you still want to merge this PR, re-open it.

See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

aviator-app · 2024-04-26T06:14:12Z

FlexReview Summary

Based on the code complexity and the author's expertise score, these are the suggested reviewers:

@draftcode (current review load: 0)

See the list of alternate reviewers in the detailed breakdown below.

Detailed Breakdown

Author’s expertise score for the modified files:

cmd/av/stack_tree.go (0.00)
internal/actions/pr.go (0.00)
internal/actions/pr_test.go (0.00)
internal/actions/sync_branch.go (0.00)
internal/config/config.go (0.00)
internal/utils/stackutils/stackutils.go (0.00)

Files	Reviewers
`internal/actions/sync_branch.go` †, `internal/config/config.go` †	`@draftcode` (score: 1.00, current review load: 0), `@doratzeng` (score: 0.31, current review load: 0), `@jainankit` (score: 0.31, current review load: 8)

† Indicates that the file doesn't need an expert review. (?)

See full breakdown of the reviewers on the Aviator webapp.

oleg-codaio · 2024-04-26T06:17:13Z

Oops - not ready for review yet. Goal is to bring this CLI on par with https://github.com/ejoffe/spr, https://github.com/facebook/sapling, https://github.com/VirtusLab/git-machete, etc. if that's something the maintainers are interested in.

jainankit · 2024-04-27T00:44:46Z

@oleg-codaio thanks for putting this together 🙏 I think it'll be easier if you break this down into smaller changes for review. After all we are working on stacked PRs :)

Let me know if you need any help.

oleg-codaio · 2024-04-27T02:37:31Z

Sounds good! I can send out smaller PRs once I've had a chance to address some issues and dogfood the feature more.

However, do stacked PRs work properly across forks? I don't think there's a way to submit (at least not now) PRs using av to a different repo, and there might also be a GitHub limitation that requires write access to properly chain them.

jainankit · 2024-04-29T22:35:57Z

You are right, it won't work well with forked repo. Feel free to continue on this change or separate out if you find a logical way to segment. We can take a look when you are ready.

oleg-codaio · 2024-04-30T18:20:47Z

I've made some more progress and fixed some other minor bugs on the way. As far as getting this reviewed, I can start sending out smaller target PRs soon.

If you have any early feedback on the stack visuals, open to it here as well. But here's how I approached it thus far:

For the majority of stacks (based on my experience with stacking and reviewing other devs' stacked PRs), they'll start at the trunk branch and create a single, linear stack of PRs (i.e., a degenerate tree). I think for those it's nice to see the bottommost branch at the top with the trunk all the way at the bottom, like so:

This is also similar to the av stack tree command shows the tree.

Once a stack gets more complex with different fork points, it becomes harder to represent the stack tree this way (unless you show sibling branches to the side somehow and prune the tree). The current implementation instead prints the output as a left-to-right tree:

Unlike what av stack tree does, this visualization isn't "sorted" to have the current branch be along the top. The reason for this is to have a consistent tree across the whole stack of PRs, so you can navigate from one branch to another more predictably.

If you have any feedback on this, let me know.

draftcode · 2024-04-30T18:48:28Z

Yep. It's hard to replicate what current av stack tree (or git log --graph) does with HTML lists. Using the second option (top-to-bottom tree) looks fine to me.

oleg-codaio · 2024-04-30T20:07:51Z

Sounds good! One other thing - instead of having the stack at the bottom of the PR, we could also have it on the top in a collapsable <detail> header. Pros are that it's easy to see the dependent PR and every time you load a different PR, the stack will be in the same place (since it won't be shifted down depending on the PR body length). ~~But a downside is it's expanded by default.~~ Could have it expanded by default too.

Any preference on one or the other, or providing an option for the user to pick their preference?

draftcode · 2024-04-30T21:26:36Z

It's fine in any cases. Please pick one. If there's a FR for choosing another way, we can add a configuration for the style.

oleg-codaio · 2024-05-01T23:54:30Z

cmd/av/commit_amend.go

@@ -78,7 +78,7 @@ var commitAmendCmd = &cobra.Command{
 		// Even if it's not configured, there's no need to fetch/push
 		state.Config.NoFetch = true
 		state.Config.NoPush = true
-		err = actions.SyncStack(ctx, repo, client, tx, branchesToSync, state)
+		err = actions.SyncStack(ctx, repo, client, tx, branchesToSync, state, actions.WithLocalOnly())


Split this out into #266.

No functional changes. Move `buildTree()` and other helper functions to a new `stackutils` module, and export `BuildStackTree()` from there. We'll later reuse `buildStackTree()` there to construct the tree of nodes needed for the stack to be written to PR bodies. See #264 for context.

As part of adding support for including the stack in PR bodies (see #264), we'll need to extend templating support to handle more than one section: ```  { stack here }  Here is the main PR body description. ``` The current `ParsePRMetadata()` implementation is efficient, but makes it hard to support another section. This PR updates `ParsePRMetadata` and `AddPRMetadata` to instead make use of substrings and `strings.Builder`. Performance isn't really a concern here - these PR bodies aren't that big and we spend a lot more time interacting with the `git` CLI anyway.

oleg-codaio · 2024-05-03T01:10:03Z

Closing; most of the changes have been merged in and this is the final one: #271

oleg-codaio and others added 8 commits April 25, 2024 21:47

Move stack utils to separate module

dccf133

Merge pull request #1 from oleg-codaio/move-stack-utils-to-separate-m…

4dcb95e

…odule Move stack utils to separate module

Support writing stack to PR description

4ad8ead

Write out stack on sync

27e3bb5

Merge pull request #2 from oleg-codaio/support-writing-stack-to-pr-de…

c63ab8a

…scription Support writing stack to PR description

Merge pull request #3 from oleg-codaio/write-out-stack-on-sync

d03a2a0

Write out stack on sync

Exclude branches on other trees

96f08ab

Merge pull request #4 from oleg-codaio/exclude-branches-on-other-trees

bffe3bf

Exclude branches on other trees

aviator-app bot requested a review from draftcode April 26, 2024 06:14

oleg-codaio marked this pull request as draft April 26, 2024 06:14

Add attribution, don't write non-trivial stacks

1716e83

oleg-codaio added 3 commits April 29, 2024 23:20

Refactor to write stacks in second pass

717d217

Update logging

8392958

Update to full stack, fix more bugs

c61200d

Alternate rendering

7180381

Open by default

133d244

oleg-codaio added 2 commits April 30, 2024 17:20

Improve top stack

49a00f2

Don't fetch/push when committing

9f9eb92

oleg-codaio commented May 1, 2024

View reviewed changes

oleg-codaio mentioned this pull request May 2, 2024

Move stack helpers to separate module for reuse #267

Merged

oleg-codaio mentioned this pull request May 2, 2024

Update av stack submit to open PRs in browser after entire stack is submitted #268

Merged

oleg-codaio mentioned this pull request May 2, 2024

Refactor PR body parsing #270

Merged

oleg-codaio closed this May 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Output stack graph to PR bodies #264

[WIP] Output stack graph to PR bodies #264

oleg-codaio commented Apr 26, 2024 •

edited

Loading

aviator-app bot commented Apr 26, 2024 •

edited

Loading

aviator-app bot commented Apr 26, 2024

oleg-codaio commented Apr 26, 2024

jainankit commented Apr 27, 2024

oleg-codaio commented Apr 27, 2024

jainankit commented Apr 29, 2024

oleg-codaio commented Apr 30, 2024

draftcode commented Apr 30, 2024

oleg-codaio commented Apr 30, 2024 •

edited

Loading

draftcode commented Apr 30, 2024

oleg-codaio May 1, 2024

oleg-codaio commented May 3, 2024

[WIP] Output stack graph to PR bodies #264

[WIP] Output stack graph to PR bodies #264

Conversation

oleg-codaio commented Apr 26, 2024 • edited Loading

Work in progress

aviator-app bot commented Apr 26, 2024 • edited Loading

Current Aviator status

aviator-app bot commented Apr 26, 2024

FlexReview Summary

oleg-codaio commented Apr 26, 2024

jainankit commented Apr 27, 2024

oleg-codaio commented Apr 27, 2024

jainankit commented Apr 29, 2024

oleg-codaio commented Apr 30, 2024

draftcode commented Apr 30, 2024

oleg-codaio commented Apr 30, 2024 • edited Loading

draftcode commented Apr 30, 2024

oleg-codaio May 1, 2024

Choose a reason for hiding this comment

oleg-codaio commented May 3, 2024

oleg-codaio commented Apr 26, 2024 •

edited

Loading

aviator-app bot commented Apr 26, 2024 •

edited

Loading

oleg-codaio commented Apr 30, 2024 •

edited

Loading