git-town icon indicating copy to clipboard operation
git-town copied to clipboard

Feat: Display lineage / hierarchy for Proposals

Open yaadata opened this issue 6 months ago • 3 comments

Addresses:

https://github.com/git-town/git-town/issues/3003

yaadata avatar May 07 '25 04:05 yaadata

You can run make fix to fix the linter errors.

kevgo avatar May 07 '25 11:05 kevgo

@Ydot19 please ping me when you want me to take another look at this. Thanks!

kevgo avatar Jun 10 '25 14:06 kevgo

Just wanted to say that I'm incredibly excited by this PR and looking forward to it getting merged!

ceilfors avatar Jun 11 '25 11:06 ceilfors

@Ydot19 Nice work so far! Let me know if there's anything I can do to help get this PR across the finish line.

kevgo avatar Jun 21 '25 11:06 kevgo

@kevgo Sorry I have been going through a rush at work / life but it is my focus to get this feature complete by July 4th weekend.

I last touched this PR about 2 weeks go but I want to break down my thought process. Below are sections

  • Realizations (what I have learned from opening up this pull-request)
  • Questions (questions I have for you @kevgo)
  • Design (my current design approach)
  • Next Steps

to help parse this large response

Realizations

  • Displaying the lineage on a proposal is best done on a proposal comment. The Proposal Body is not a good location to display this information because if you create future child proposals at a later time, it is hard to update and won't play nice for organizations using proposal templates for the proposal body.

This brings up interesting challenges of how to reliably update the same comment if it still exists.

  • Displaying the lineage on proposal creation is not a reasonable workflow because creating a proposal relies on opening up the browser and completing the flow on the browser. There is no hook mechanism for listening to proposal. The approach I am considering is moving the ability to create the proposal comment as a part of the git-town sync command, initially behind a flag called -l (--lineage)

  • Tangentially, we should be displaying the same lineage information on the terminal (something like git-town branch -p, shows the lineage as it would appear in a proposal). I am currently considering this as the v0 to prove that lineage information is correctly being generated. This I want to introduce in a separate pull-request.

Current Questions:

  • This method seems to be the preferred way to get branch lineage. Let me know if there is another way that is preferred
func (self Lineage) BranchLineageWithoutRoot(branch gitdomain.LocalBranchName, perennialBranches gitdomain.LocalBranchNames) gitdomain.LocalBranchNames {
	if self.Parent(branch).IsNone() {
		result := gitdomain.LocalBranchNames{}
		if !perennialBranches.Contains(branch) {
			result = append(result, branch)
		}
		return append(result, self.Descendants(branch)...)
	}
	return append(append(self.AncestorsWithoutRoot(branch), branch), self.Descendants(branch)...)
}
  • How do you propose we store the comment id containing the lineage comment for updates to lineage information. I am currently going to implement a 'dumb' approach, which is to store a hidden mark down text and search the first 10 comments on a proposal for this text (see the design section for more on this). If we find this hidden text in a comment, we will update the comment; otherwise, we will create a new lineage comment

Design

Commands

git-town sync -p

Purpose: Syncs and creates the lineage comment in proposals

git-town sync -s -p

Purpose: Syncs and creates the lineage comment across the entire stack of proposals

git-town branch -p

Shows the lineage comment in the terminal for existing proposals. If a branch has no proposal, it will show that there are no proposals for the branch

Comment Format:

<!---GIT-TOWN GENERATED--->
This proposal is part of a stack. 

main
    ↳ branch_name_a ➜  [#prosopalID](link to proposal)
        ↳  branch_name_b ➜  [#prosopalID](link to proposal)
            ↳ branch_name_c ➜  [#prosopalID](link to proposal)  ***CURRENT*** 

NOTE:

  • The current surrounded by *** denotes that the reader is viewing that proposal
  • The hidden text is used to the existing lineage comment if we need to update the information
  • If there is a branch that exists locally but there is no proposal for it, it will not appear in the proposal comment. Example: take the comment above, if there is also a branch called branch_name_d that exists locally but no proposal exists on the forge, then it will not appear in the comment

Test Cases

These are the cases I am considering prior to considering this feature ready. Let me know if there are others

  • Generating the base case (see the design section). This is the happy case where a proposal and its entire lineage of proposals are created at the same time.
  • Generate comments when there is a perennialBranch in the lineage. If there is a perennial Branch, the perennial branch will not have a proposal linked to it in the lineage comment

Example:

<!---GIT-TOWN GENERATED--->
This proposal is part of a stack. 

main
    development (perennial branch)
       ↳ branch_name_a ➜  [#prosopalID](link to proposal)
           ↳  branch_name_b ➜  [#prosopalID](link to proposal)
               ↳ branch_name_c ➜  [#prosopalID](link to proposal)  ***CURRENT*** 
  • Update lineage comments when new proposals are created that are part of the lineage of branch. For example, say we have two branches (part_a and part_b) that are created and you are currently viewing part_b's proposal BEFORE:
<!---GIT-TOWN GENERATED--->
This proposal is part of a stack. 

main
    ↳ part_a ➜  [#prosopalID](link to proposal)
        ↳  part_b ➜  [#prosopalID](link to proposal) ***CURRENT*** 

If a part_c proposal is created at a later time, running git-town sync -p should update this comment to be the following

AFTER:

<!---GIT-TOWN GENERATED--->
This proposal is part of a stack. 

main
    ↳ part_a ➜  [#prosopalID](link to proposal)
        ↳  part_b ➜  [#prosopalID](link to proposal) ***CURRENT*** 
            ↳ part_c ➜  [#prosopalID](link to proposal) 

Similarly, if you update the entire stack, the lineage comment for part_a should also be updated

  • Not displaying branches in the lineage comment that do not have a proposal

Next Steps:

  • This pull-request will focus only on enabling git-town sync -p for the initial comment creation
  • Separate pull-request to enable updating an existing lineage comment on a proposal
  • Separate pull-request: Enabling git-town sync -s -p
  • Separate pull-request: Enabling git-town branch -p

yaadata avatar Jun 28 '25 04:06 yaadata

@Ydot19 thanks for the update, and for all your great work and thoughtful ideas on this!

I recommend aligning your implementation as closely as possible with how the Git Town GitHub Action already does this. There are a few strong reasons for doing so:

  • The team maintaining the GitHub Action has already explored and iterated on multiple approaches before landing on the current design. Leveraging their experience lets us stand on the shoulders of giants here.

  • It ensures a consistent UX and interoperability. Whether somebody uses this feature via the Git Town CLI or through the GitHub Action, the lineage formatting and behavior should be the same. For example, if a team isn't using the GitHub Action yet, but someone starts including the lineage in proposals through the CLI, and later the team adopts the action, the transition should be seamless without visual or behavioral differences.

I think that embedding the stack into the proposal body has strong UX advantages for many (most?) users, so we should try to make this happen even if it doesn't work in some edge cases. If some of those edge cases are a big enough problem, we can layer on additional functionality to make things work there as well. You are right that one such edge case are GitHub pull request templates. A couple of potential paths forward here:

  • Convince the affected teams to include Git Town's invisible placeholder into their PR templates. If they find the lineage useful, they'll have no problem doing this.
  • Users manually insert the placeholder when creating proposals. Not ideal, but also not impossible.
  • Optionally, when we get user feedback from the real world around this, also support displaying the stack lineage in comments.

I also really like your idea of updating the lineage during git town sync. That seems like a natural fit. I think a preference would be a good way to control this behavior consistently in all affected commands like git town sync, git town ship, git town delete, git town append, git town prepend, etc.

Let's implement the lineage functionality in each of those commands one by one (using separate proposals), so that we keep momentum and the respective PRs remain easier to review.

Finally, for the lineage data: the Lineage.BranchLineageWithoutRoot method returns the right structure for this use case. To include the root branch in the output (which makes sense for display purposes), consider introducing a new method that preserves it, like Lineage.BranchLineage.

kevgo avatar Jun 29 '25 22:06 kevgo

@kevgo I do believe using a proposal comment similar to graphite (here is one I pulled off line as an example) is going to be the preferable approach.

The way that graphite team does it is very similar to how the git-town action works in the proposal body. I do think we should stick with proposal comments because this is already on the action's team roadmap to pivot to a comment https://github.com/git-town/action/issues/28

yaadata avatar Jul 01 '25 00:07 yaadata

@Ydot19

Displaying the lineage on a proposal is best done on a proposal comment.

I don't necessarily agree with this. Each team is unique with their own set of processes & preferences, so it's ultimately up to them to decide what is "best". The plan for https://github.com/git-town/action/issues/28 is to support both the proposal body and comment as a visualisation target via config -- IMO the CLI should do the same.

Notes on Design

I think the key design challenge when syncing lineage via CLI will be aligning forge lineage state with local lineage stage. Proposals can be modified/merged/closed independently from Git Town, which means the local lineage may not represent the actual state of the repository. This means that the forge is ultimately the source of truth when it comes to lineage data. If we want to sync lineage visualisations via CLI, the solution we come up with will need to take this into account.

Tangential to aligning forge & local state, another design challenge will be ensuring our solution works when multiple contributors work on the same stack/hierarchy. Storing comment IDs locally may work with a sole contributor, but depending on how it's implemented this may not adapt correctly for multiple contributors. In my experiencing, leveraging the forge as the source of truth is the usually the best bet here. My first instinct in this scenario would be to pull proposal & comment data upfront before executing a sync and then search for the invisible anchor in order to locate the visualisation.

I also question the DX improvement syncing via CLI would provide 🤔. Pulling forge data in order to generate correct lineage visualisations is going to introduce additional networking overhead which slows down syncing process. A big advantage of Git Town is how snappy due to it mostly wrapping the Git CLI. Some questions I find myself asking here are:

  • Would our efforts perhaps be better directed towards creating forge-specific integrations (like the GitHub Action) that offload this to the forge?
  • What would be the advantages of implementing this via CLI instead?

tranhl avatar Jul 01 '25 01:07 tranhl

The GitHub actions approach works if you are the owner of the repository or on a team/org that is okay with updating there PR template for this use case.

In my mind, the graphite approach allows individuals to gently introduce stacking to their proposals before getting buy-in about the tool via the comments approach.

Ultimately, I won't die on this hill. If we want consistency I can do exactly what the action does via cli OR if we don't want this functionality in git-town I can also forego this functionality

yaadata avatar Jul 01 '25 01:07 yaadata

It seems like I've come across as against syncing via CLI overall -- that was not my intention! The need for a non-invasive alternative to the GitHub Action is pretty clear. What I'm really saying is that the solution we come up with needs to be 1. behave correctly and 2. introduce minimal performance impact to the core Git Town experience.

I don't think 1. can be achieved without pulling forge data directly, but doing this without impacting 2. is not clear to me yet.

tranhl avatar Jul 01 '25 01:07 tranhl

Forge vs CLI for updating stack lineage embedded in proposals

I agree that a forge-based solution - like our GitHub Action - is the preferred approach, and it's what Git Town recommends when feasible. The CLI-based approach is for situations where setting up the GitHub Action isn't possible: when decision-makers aren't (yet) on board, or when the team uses a forge that doesn't (yet) have a supported CI job.

The CLI-based stack lineage in proposals is also a natural way to raise visibility of Git Town in teams who aren't familiar with it.

Stack lineage in proposal body vs. comments

There are valid use cases for either approach, and ideally we should support both. https://github.com/git-town/action/issues/28 is about adding support for displaying stack lineage in a comment. It doesn't remove the existing behavior of including it in the PR body.

Updating lineage embedded in proposals via CLI requires (1) accurate lineage info from the forge, and (2) introduces additional API calls that may slow down the CLI

Totally agree. One way to handle this might be to configure which approach the team uses via a configuration setting. That way, teams who use the GitHub Action won't be impacted by the additional overhead.

One the other hand, keeping the local Git Town client up to date on lineage changes made on the forge sounds like a good thing in general...

kevgo avatar Jul 01 '25 15:07 kevgo

Thank you for the clarity. I will revise this proposal to update the proposal body similar to the github action for functional and ui parity.

Let me work through a functioning prototype then we can discuss the more thorny topics/issues with this proposal

yaadata avatar Jul 01 '25 19:07 yaadata

@kevgo I now have a working feature for this change. I want to add more tests but this PR is already at 800+ lines so this can be a separate changes to follow

Please feel free to pilot the changes and take it for a test drive.

yaadata avatar Jul 27 '25 06:07 yaadata

I have more branches that build of this branch that adds more enhancements and fixes to this feature. Because I don't have access to make branches to the git-town repository I cannot show the proposals queued off this branch

I have a mirror of this pull-request on my fork https://github.com/Ydot19/git-town/pull/2 and it shows more prs ready for review

Likely the next enhancement I will make is make an update to a stack of proposals in one command.

yaadata avatar Jul 28 '25 10:07 yaadata

You can run make fix to fix the linter errors.

kevgo avatar Jul 28 '25 15:07 kevgo

@kevgo it seems that the generation script is not behaving the same on my local machine because there is no diff I see.

Please see https://github.com/git-town/git-town/pull/5283#discussion_r2236948622

yaadata avatar Jul 28 '25 15:07 yaadata

updated and responded in the other thread https://github.com/git-town/git-town/pull/5283#discussion_r2237613394

yaadata avatar Jul 28 '25 19:07 yaadata

To help get this PR shipped, I have just shipped #5291. It provides a configuration setting for how lineage should be added to proposals. If this setting is set to configdomain.ProposalsShowLineageCLI, then the following Git Town CLI commands should update the lineage in the proposal:

  • propose
  • sync
  • swap
  • append
  • prepend
  • delete
  • ship
  • merge
  • swap

Let's implement the changes for each of these commands as a separate pull request.

I'm not convinced that there should be CLI flags that configure whether the Git Town command should update lineage in proposals or not. This leads to inconsistent behavior:

  • I create a proposal via git town propose --lineage
  • but then when syncing I forget to add --lineage, and now the proposal displays an outdated lineage.

Although a user error, this makes Git Town look bad. Good tooling should make it easy to do the right thing, and make it hard/impossible to do the wrong thing. I therefore think, in this case a config option is the right way forward.

Testing this out is easy:

git config set git-town.proposals-show-lineage cli  # enable the feature for testing
git propose                                         # creates a proposal with lineage
git prepend other-branch                            # updates the lineage in the existing proposal
git config unset git-town.proposals-show-lineage    # disable the feature

kevgo avatar Jul 29 '25 13:07 kevgo

@kevgo want to give this another look?

yaadata avatar Aug 04 '25 13:08 yaadata

git config set git-town.proposals-show-lineage cli  # enable the feature for testing
git propose                                         # creates a proposal with lineage
git prepend other-branch                            # updates the lineage in the existing proposal
git config unset git-town.proposals-show-lineage    # disable the feature

~do you have a special setup that you're running locally because I don't have access to setting config~

~/repositories/git-town on  support-pull-requests-comments [$] 
🕙[ 10:19:00 PM ] ❯ git-town config set git-town.proposals-show-lineage cli

Error: unknown command "set" for "git-town config"

~I was able to test by remove a conditional if on this particular config value~

EDIT: I see that you made your git-town commands available to git and this are the git tags themselves

yaadata avatar Aug 05 '25 03:08 yaadata

It's git config set git-town.proposals-show-lineage cli, without "town".

kevgo avatar Aug 05 '25 20:08 kevgo

Added a commit for each review comment to make it easier to follow. Will do one more round of manual testing tomorrow.

Also, I don't understand why the endtoendtest are failing.

If there are other smaller tweaks or adjustments in code style needed, you can push to this branch if it helps reduce the feedback cycle

yaadata avatar Aug 07 '25 06:08 yaadata

Alright, merging this in. Thanks so much @Ydot19 for the hard work to get this pretty complex feature built and polished. You rock!

kevgo avatar Aug 09 '25 12:08 kevgo

@kevgo thanks, I don't know what the expectations are moving forward or whether you want to drive the rest of the changes listed here https://github.com/git-town/git-town/pull/4871#issuecomment-3132610538 by you.

then the following Git Town CLI commands should update the lineage in the proposal:

propose
sync
swap
append
prepend
delete
ship
merge
swap

If you want to drive the rest, that sounds good to me (and maybe faster). If not, I can continue contributing to drive this to completion but would like to understand the sequence as to not duplicate work you are planning to execute

yaadata avatar Aug 09 '25 19:08 yaadata

Thanks for coordinating on this. I’m currently focused on #5156 and a few other high-priority items, so I won’t be able to work on the feature you’ve built here for a while. I'd love for you to keep pushing it forward and get it to a releasable state.

Adding proposal lineage to other Git Town commands should be much smoother now that the heavy lifting is done. You just need to hook it into the other commands.

I may contribute a few tweaks to align the code more closely with the style and architecture of the rest of the codebase. Those changes shouldn’t interfere with your work, but if they do, I’m happy to prioritize your changes to avoid merge conflicts.

Does this work for you?

kevgo avatar Aug 09 '25 22:08 kevgo

This works for me I'll continue onward!

yaadata avatar Aug 10 '25 00:08 yaadata