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

⭐️ Lean Branching - a git branching model to keep clean history #1432

Open
DanPristupov opened this issue Jan 20, 2022 · 66 comments
Open

Comments

@DanPristupov
Copy link
Contributor

We are thinking about adding to Fork a feature which would help teams to keep clean history.

As you know it's pretty difficult to have a clean history in git. I've got some ideas and tricks how that can be achieved. Briefly: create a separate branch for each feature, synchronize your changes with the main branch using rebase and merge back to main at the end. My point of view is similar to what Chris Manson suggests in his great article (https://simplabs.com/blog/2021/05/26/keeping-a-clean-git-history/, I really recommend to read it if you haven't yet).

Here's an example of a decent git history:

Screenshot 2022-01-20 at 15 58 13

The commit history may look simple and clear, but making it so is surprisingly difficult and requires the use of some advanced (and dangerous) git features. In particular:

  • rebase (to rebase on main)
  • force push with lease (to update the remote tracking reference after the rebase)
  • hard reset (if something went wrong and we want to start from scratch).

We are thinking about adding to Fork the ability to easily make those operations and perform dangerous things automatically (without hiding them, though!). Something like git-flow, but not so complicated and with a dedicated sync option. We called it "Lean branching" trying to reflect the approach but may be there could be a better name.

Goals:

  • Be simple
  • Keep history clean by default
  • Do not introduce a naming convention. Fork can work with main, develop or master.
  • Intuitive for advanced users
  • Easy to explain to non-developers

The workflow is supposed to be like the following:

  • Start a branch on main
  • Commit changes. Sync with main as often as possible to minimize future conflicts
  • When the work is done, finish the branch by merging it into main

So, there are just 3 actions:

Screenshot 2022-01-20 at 16 33 42

1. Start new branch
Create and checkout new branch on main or origin/main (depends on which one is ahead)

2. Sync active branch

  • rebase on the remote tracking branch (if needed)
  • rebase on main (if needed)
  • restore the remote tracking branch position on the new location using force push. So, if it was 2 commits behind the branch, it will remain 2 commits behind after the sync

If the active branch is main, sync will simply rebase it on origin/main

3. Finish active branch

  • merge the current branch into main, but use fast-forward if the branch consists of a single commit.
    precondition: main must be in sync with origin/main

Simple example:

I've finished working on FW-433 and want to merge the changes into master:
Screenshot 2022-01-20 at 16 10 40

Sync FW-433 with master first:
Screenshot 2022-01-20 at 16 26 53

Finish FW-433:
Screenshot 2022-01-20 at 16 27 16

Now I can test if everything works properly and then push master.

A few notes:

  • Your changes will not be pushed or fetched automatically. You must do that yourself, same as you do now.
  • All 3 actions are available in the Branch dropdown on the toolbar in Fork
  • The underlying git commands can be seen in the activity manager if you click on the status control on the toolbar

Tanya and I have been using this feature 'as users' for some time and it works really well for us.

It would be great if you could try it and let us know what you think.

You can download the build here: https://cdn.fork.dev/prerelease/ForkWin-1.69.10.zip. Extract it somewhere and run fork.exe.

Use with caution, it's still in beta.

@DanPristupov DanPristupov pinned this issue Jan 20, 2022
@Calneideck
Copy link

I'm running 1.69.10 but I can't see the new actions in the branch dropdown or anywhere else. Do I need to do something to activate them?

@DanPristupov
Copy link
Contributor Author

@Calneideck
I just checked the source code. The items appear under the following conditions:

  • there is local branch named main, develop or master
  • the local main branch must have the remote tracking reference.

Is it possible that your local master doesn't have a tracking reference (upstream) to origin/master? You can change that in the context menu for local branch on the toolbar.

Screenshot 2022-01-21 at 8 33 37

@Calneideck
Copy link

@DanPristupov Thanks, that was it! I had main that for some reason was not tracking origin/main.

@mloskot
Copy link

mloskot commented Jan 28, 2022

Is this the branching model that has just been released with the Fork 1.70.0?
Can be closed, I guess.

@DanPristupov
Copy link
Contributor Author

@mloskot let's keep it open for now. We are still looking forward to hear feedback from people who tried it :)

@ghost
Copy link

ghost commented Jan 29, 2022

I gave it a quick try. I really like this git methodology in general.
I usually interactive rebase, squashing as needed on the target branch.

I am used to working with branch protections, disabling any pushes to origin/main and enforcing FF-only merges in the git hosting service.
I think this will help folks who may not be keen to dive into advanced git usage.

I don't see myself using this in day-to-day operations but I can see it being helpful for others.

@lievendf
Copy link

I like it but I think it should be possible to do the actions while there are uncommitted changes as well, which is the case in most of my repos.
Having to manually stash these first, while the "manual" branch/commit/rebase/merge has the option to keep them there (stash and reapply automatically) feels like a step back...

@DanPristupov
Copy link
Contributor Author

@LievenDeFoor this is a good point. I agree, Sync must also 'stash and reapply' when needed.

@lievendf
Copy link

lievendf commented Jan 31, 2022

* Do not introduce a naming convention. Fork can work with `main`, `develop` or `master`.

Could these names be made configurable?
We have long lived branches for released versions of our product that receive frequent updates.
Currently these can't use "Lean branching" as the names are "ProductName_MajorVersion" or similar...

@jnunnerley
Copy link

jnunnerley commented Feb 1, 2022

For 'force push with lease' to work correctly don't you need to disable automatic fetching ('Fetch remotes automatically' in General config) ?

@DanPristupov
Copy link
Contributor Author

@jnunnerley may be I'm missing something, but I think automatic fetch is not related. --force-with-lease just checks that the remote branch on server is still on the commit as we expect it to be on. It's just a protection to avoid race conditions.

@jnunnerley
Copy link

@jnunnerley may be I'm missing something, but I think automatic fetch is not related. --force-with-lease just checks that the remote branch on server is still on the commit as we expect it to be on. It's just a protection to avoid race conditions.

The following details the issue

https://blog.developer.atlassian.com/force-with-lease/

Do you do the workaround?

@lievendf
Copy link

lievendf commented Feb 2, 2022

Another remark (sorry!):
We never use fast-foward merges and always create a separate commit when merging branches.
The "Finish branch" does not have this option and always does a fast-forward merge.
Can this be added as an option?

@Francoimora
Copy link

I was loving it until last update that made the sync operation mandatory. For non-dev users, rebase is harder to understand than merging (and it makes the history unclear for us). Could you let an option to let us merge without rebasing (like in the beta build) ?

@DanPristupov
Copy link
Contributor Author

@jnunnerley do you mean the race condition, when new remote branch position could be fetched right before the push and would be overwritten? It shouldn't be a problem, because we can use --force-with-lease=remoteBranch:expectedSHA. https://git-scm.com/docs/git-push#Documentation/git-push.txt---force-with-leaseltrefnamegt

@LievenDeFoor

We never use fast-forward merges and always create a separate commit when merging branches.

Is there any real reason behind that? Having 2 commits for a single commit change definitely contradicts with the original goal 'clean history' and even with the name (it's not a lean branching).

@Francoimora

For non-dev users, rebase is harder to understand than merging (and it makes the history unclear for us).

Then, why do you want to use 'lean branching' and not just merge (or even gitflow)?

Could you let an option to let us merge without rebasing (like in the beta build) ?

I don't remember it working that way. If it was, I guess, it was a bug. The whole point of the lean branching is to rebase before the merge.

@Francoimora
Copy link

@DanPristupov The main problem with using gitflow with fork right now (for non dev users) is that it's really easy to start a feature branch, but really hard to finish it (we need to search for it in quick launch). The lean branching added the "Finish feature" option that we use on our workflow, directly on the branch button.

@DanPristupov
Copy link
Contributor Author

@Francoimora I'll have a look if we can add the 'finish feature' for gitflow.

@lievendf
Copy link

lievendf commented Feb 3, 2022

@LievenDeFoor

We never use fast-forward merges and always create a separate commit when merging branches.

Is there any real reason behind that? Having 2 commits for a single commit change definitely contradicts with the original goal 'clean history' and even with the name (it's not a lean branching).

Company policy. No direct commits on master/develop/main... are allowed to be pushed, except for merge commits.
I must say that I personally prefer this approach as well, as you can clearly see branches for features being started/merged, while a fast-forward commit loses this (visual) information...

@rmarskell
Copy link

I love this idea. I've been training my staff to work like this for years now, but due to the risks inherent with rewriting history, it's always had a bit of a learning curve. We do this on develop and then use merge commits on master to indicate when releases went to production. It makes it very clear what work went into a feature, and what features went into a release. For example:
Orange = main
Yellow = develop
Red = feature

image

I agree with others that it would be nice to have the branches configurable (perhaps in the repository settings?).

Would it be possible / make sense to put this under the right-click menu as well (perhaps also based on config)?

@DanPristupov
Copy link
Contributor Author

@rmarskell thank you for sharing your use case.

We do this on develop and then use merge commits on master to indicate when releases went to production.

I also noticed a bug, that 'develop' must be a higher priority than 'main'. So, in your case 'develop' must be used by default.

I agree with others that it would be nice to have the branches configurable (perhaps in the repository settings?).

Yes. In the next update we will make the main branch configurable.

@rmarskell
Copy link

I also noticed a bug, that 'develop' must be a higher priority than 'main'. So, in your case 'develop' must be used by default.

I see what you mean. Yes, I suppose there would need to be some idea of which takes precidence. This rebasing structure doesn't really work with multiple levels (i.e. rebasing develop onto main, if it had changes, would lose the benefit of being able to see feature branch history since I think the rebase flattens the merge commits). If you can configure the branch to use for rebase/merges in the repository settings (so you can work on differently structured repos), that should work fine for us. :)

Yes. In the next update we will make the main branch configurable.

🎉

@chastelain
Copy link

Good thing!
If you need some feedback, I've been working with this lean branching in a trunk-based development for years (and using Fork everyday from its beginnings ❤️)
Only merge commits are allowed in master, need to be approved within a PR, and have to be up-to-date (rebased with latest master

image

What I would appreciate would be optional automatic mechanisms to reduce side effects in inexperienced users.
I would say, for example, auto-fetch & pull rebase whenever those lean branch actions are triggered (and to be optional, like beginner-friendly check case).

Another thing, as you can see in my picture, we use the merging branch option directly from Github as it autogenerates the merge commit message with the PR id and branch name, and allow easily to input its description in the commit body message.
The only way to do it in Fork is something I found not to be very intuitive: amending the merge commit to reword it.
image
A popup or whatever could be proposed when merging.

Good work by the way, really appreciating it, cheers!

@Vankog
Copy link

Vankog commented Feb 21, 2022

I like it but I think it should be possible to do the actions while there are uncommitted changes as well, which is the case in most of my repos. Having to manually stash these first, while the "manual" branch/commit/rebase/merge has the option to keep them there (stash and reapply automatically) feels like a step back...

BTW: This also affects interactive rebase. There, the missing stashing option is bugging me as well. ;-)

We never use fast-foward merges and always create a separate commit when merging branches.
The "Finish branch" does not have this option and always does a fast-forward merge.
Can this be added as an option?

This is a highly opinionated topic.
I myself also refrain from doing a merge commit if the branch only has a single commit. It makes the history unecesarily convoluted.
However, there are folks that view it the other way around.
Frankly, in general I don't like merge commits at all, as they can hide changes that were necessary due to conflicts etc. Issues that arise due to this are notoriously hard to find.
Especially, because Fork does not support the "--full-history" flag to show merge commits in file history. (TortoiseGit recently implemented it: https://tortoisegit.org/issue/3728)

And while Github or Bitbucket do allow the option to switch this behavior for each and every PR/MR. Gitlab does not allow this. There you can only set the general setting to either only allow fast-forwards or only merge-commits.

@skstrobel
Copy link

Frankly, in general I don't like merge commits at all, as they can hide changes that were necessary due to conflicts etc. Issues that arise due to this are notoriously hard to find.

I also dislike changes that get made during a merge. My personal practice is to merge main into my feature branch immediately before merging my feature branch back into main and closing it. The first step makes it clear which changes were needed to integrate the changes made in main into my feature branch. I can test that integration and make any necessary changes while still in my feature branch. After that, merging my feature branch back into main is trivial (no conflicts) and shows only the changes needed to implement the feature (without any conflict resolutions mixed in). My git knowledge is pretty shallow, so there might be a better way.

@calle2010
Copy link

calle2010 commented Feb 26, 2022

The only thing that I dislike about this whole new feature is how it makes fork an opinionated tool towards a specific workflow. By putting this near the center of the header it will make some individuals and teams believe that this is "the" way to work with Git and everything else is "too complicated". I like fork especially because it allows me to do the same things as on the command line, mostly with a better UI, without restricting me too much.

Every other tool I tried so far exposed me to different terminology than Git itself or promoted ways of working that only made sense in a specific setting. This makes it hard to even talk and reason about different aspects of Git workflows if knowledge of developers is only based on a specific UI instead of Git itself.

When I tell a team member that they have to "sync" their branch with main I want them to follow our workflow, not accidentally find something in a tool they don't fully understand.

I appreciate the idea to give fork the support for different workflows. It would be great if this was an optional (as in opt-in) and pluggable feature, perhaps as a set of predefined custom commands. Then I could decide to use it or not, and otherwise not be bothered with it. And I could define my own workflow and reach the required menu items easily.

@JosephTLyons
Copy link

The new feature seems cool, my only issue is it assumes you work entirely off of master. Our sprints come off of master and our user stories come off of sprints. So for this to work for that sort of model, we'd need to be able to configure the branch we start, sync, and finish with. Instead of master, we'd be starting, syncing, and finishing on our user story branches with our sprint branches. Can a setting be put into place to configure master / main to be any branch in the repo?

@DanPristupov
Copy link
Contributor Author

@JosephTLyons we will the ability to change the main branch in the next update (Fork 1.71 Windows).

@ghost
Copy link

ghost commented Mar 12, 2022

@DanPristupov
This feature is great! Could Fork add hot keys for the 3 actions?
image

@Moily
Copy link

Moily commented Aug 16, 2022

This looks to be a very useful feature and it will help massively with our standards & practices.

Could I request a setting to remove the branch once it has been finished, please?

@DanPristupov
Copy link
Contributor Author

No, I believe, the user must push and delete the branches manually and explicitly. It should not be an automatic action.

@Moily
Copy link

Moily commented Aug 17, 2022

Ok. It looks like this was designed to solve a different problem than what we need it for, in that case.

@bilke
Copy link

bilke commented Aug 26, 2022

I love this feature and suggest to make this even more prominent in the gui: the sync-action could be added to the top-level fetch, pull, push buttons.

@deuterium2h
Copy link

I love this feature! It enforces a good practice for keeping the commit history clean :)
Also, would it be possible to have an alias for main and develop branches? Since each developer (or team) has a preferred name for their main and development branch

  • origin/main or main will be called origin/my-branch or my-branch
  • origin/develop or develop will be called origin/feature/addition or feature/addition

@DanPristupov
Copy link
Contributor Author

@deuterium2h

Also, would it be possible to have an alias for main and develop branches?

Repository -> Settings for this repository -> Lean Branching -> Main Branch

There's no develop, though.

@deuterium2h
Copy link

@deuterium2h

Also, would it be possible to have an alias for main and develop branches?

Repository -> Settings for this repository -> Lean Branching -> Main Branch

There's no develop, though.

Oh! Thanks for pointing this out, I haven't explored much in repository settings ☺️

@lievendf
Copy link

@DanPristupov any update on the "stash and reapply" on finish feature (in addition to on sync)?
Currently the feature is unusable as we almost always have some uncommitted changes lingering.
A manual merge will stash and reapply, so I would expect the "finish branch" feature to have the same capabilities.
Thanks,
Lieven

@DanPristupov
Copy link
Contributor Author

@LievenDeFoor I think working directory must be clean before both 'merge branch' and 'finish feature' operations.

@lievendf
Copy link

lievendf commented Sep 21, 2022

@DanPristupov It surely doesn't have to be clean for 'merge branch' unless Fork is doing a stash before and reapply after merge automatically.
For 'finish feature' I expected the same possibilities as 'merge branch' which doesn't seem to be the case.

@HongliangQiu
Copy link

@DanPristupov
Hello

There is one problem when I "Sync" now:

When I sync local branch on release, Fork alarms "fatal: invalid upstream" error message.
When I sync remote branch on release, Fork alarms "Failed to parse SHA in ''" error message.

image

@DanPristupov
Copy link
Contributor Author

@HongliangQiu can you send the log file %localappdata%\fork\logs\fork.log to support@fork.dev, please?

@HongliangQiu
Copy link

@HongliangQiu can you send the log file %localappdata%\fork\logs\fork.log to support@fork.dev, please?

Ok, please wait a minute.

@HongliangQiu
Copy link

@HongliangQiu can you send the log file %localappdata%\fork\logs\fork.log to support@fork.dev, please?

Ok, please wait a minute.

@DanPristupov I have send one email, please check it.

@DanPristupov
Copy link
Contributor Author

@HongliangQiu thank you for the bug report! I've just released Fork 1.87.1 hotfix.

@HongliangQiu
Copy link

@HongliangQiu thank you for the bug report! I've just released Fork 1.87.1 hotfix.

Ha, I have downloaded the latest version, everything is ok now.
Thank you.

@borrillis
Copy link

I know this feature has been out for a bit now, but I just discovered it since I was about to file a new issue and saw this as a pinned issue. Having the branch button to the right of the Repository status(?) block is not very discoverable to me, I would have expected to find it on the left with the other repository actions, like so:
image

@sunghwan2789
Copy link

sunghwan2789 commented Oct 17, 2023

Can I also create a pull request on main branch with Lean Branching?

@HongliangQiu
Copy link

@DanPristupov
Hello
I have one problem when I "Sync" now:

When I Sync A branch, Fork encounters Merge Conflict.
But In fact, there is no confilct, should not show the conflic window.

image

image

fork.log

@DanPristupov
Copy link
Contributor Author

@HongliangQiu it's probably a conflict when merging the local changes that were stashed before sync.

@HongliangQiu
Copy link

@HongliangQiu it's probably a conflict when merging the local changes that were stashed before sync.

My local workspace is clean.
So, how can I solve the problem?

@DanPristupov
Copy link
Contributor Author

OK, it's difficult to say what's going on. Try to open Console and reset the sync by running rm -rf .git/fork/sync in the command line. Then try to sync again.
I think that should solve the problem.

@HongliangQiu
Copy link

OK, it's difficult to say what's going on. Try to open Console and reset the sync by running rm -rf .git/fork/sync in the command line. Then try to sync again. I think that should solve the problem.

It works. Thank you very much.

@ipsquiggle
Copy link

I just found the 'lean branch' options, this has been my workflow precisely for forever, and I've been using Fork forever, sad for me that I was unable to put the two together until now hahah. Thank you for adding this! 😁

However, I just noticed these commands don't show up in the ctl-p menu, would it be easy to add them there? Or, giving them a keyboard shortcut as mentioned above? Basically, it's the kind of thing I'd naturally trigger from the keyboard, as with pulling, to quickly make sure I'm always up-to-date, so some keyboard-centric trigger would be great...

@mAcf00bar
Copy link

I'm using fork since before the lockdown and got it approved for the whole team back in 2019. I'm still using it personally (my own licence since I do support great tools) and use it as my sole git client. It's just awesome.

Regarding the "lean branching", I like this very much, as this is sort of the mode of operation I prefer in our trunk-based model. The only whish I have: can the lean branching also be applied to an already existing feature branch (the initial branch get's created off main but through a jira ticket) and instead of finishing the whole thing with a direct merge into main, could this just push-force-with-lease to origin and create a PR (in azure devops) which then will be merged into main anyways if approved?

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

No branches or pull requests