Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6/15] feat(submit): make forges branch-agnostic #1225

Closed

Conversation

arxanas
Copy link
Owner

@arxanas arxanas commented Feb 11, 2024

Stack:


feat(submit): make forges branch-agnostic

It was previously assumed that all forges would want to create and operate on branches, and, in fact, the calling git-branchless-submit code would even create the branches returned by CreateStatus. This has turned out not to be the case. For example, the Phabricator forge embeds the commit identifier in the (updated) commit message, and doesn't need a branch (and, in practice, it's turned out to be more annoying than useful to create the branches as shorthand identifiers).

This commit removes the auto-branch-creation behavior and updates the terminology to be branch-agnostic and refer to "commits" instead of "branches".

As a result, commits are now "submitted" at the beginning of their submission lifecycle, and future pushes (etc.) are "updates". This doesn't exactly make sense with the name of the --create flag, since we never refer to "creating" a commit anywhere, only "submitting" one, so we might want to change that flag name or terminology in the future.

In doing this, I improved some of the comments/assertions on the consumer interface that became apparent when implementing the example.

I found some bugs by hand while testing and proptest also found the same bugs.
I forgot that we have a container type for the success+failure bounds, so use that here.
So that we can use this logic in `git-branchless-submit`, which wants to be fundamentally aware of commit stacks as part of submitting.
So that we can call e.g. `Repo::create_branch` and ignore the error if the branch already exists.
It was previously assumed that all forges would want to create and operate on branches, and, in fact, the calling `git-branchless-submit` code would even create the branches returned by `CreateStatus`. This has turned out not to be the case. For example, the Phabricator forge embeds the commit identifier in the (updated) commit message, and doesn't need a branch (and, in practice, it's turned out to be more annoying than useful to create the branches as shorthand identifiers).

This commit removes the auto-branch-creation behavior and updates the terminology to be branch-agnostic and refer to "commits" instead of "branches".

As a result, commits are now "submitted" at the beginning of their submission lifecycle, and future pushes (etc.) are "updates". This doesn't exactly make sense with the name of the `--create` flag, since we never refer to "creating" a commit anywhere, only "submitting" one, so we might want to change that flag name or terminology in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant