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.

Sign up for GitHub

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

Jump to bottom

Add Merge Style: Fast-Forward Only (--ff-only) #24906

Closed
chrisnc opened this issue May 24, 2023 · 10 comments · Fixed by #28954
Closed

Add Merge Style: Fast-Forward Only (--ff-only) #24906

chrisnc opened this issue May 24, 2023 · 10 comments · Fixed by #28954
Labels
proposal/accepted We have reviewed the proposal and agree that it should be implemented like that/at all. type/feature Completely new functionality. Can only be merged if feature freeze is not active.

Comments

@chrisnc
Copy link
Contributor

chrisnc commented May 24, 2023 •
edited
Loading

Feature Description

For projects that want to enforce a linear history, supporting fast-forward-only in the PR UI is very helpful. This is a similar request to #20769 except hopefully more narrow and easier to implement and explain. Rebase+fast-forward has a number of drawbacks for some workflows that are solved by fast-forward-only: mainly that commit hashes can never be modified, and also that any GPG signatures on the commits will be preserved. (Personally, if I had --ff-only, I would not need the --ff style merge as in #20769; I could choose the --no-ff merge when I don't mind a merge commit, or sync and merge --ff-only if I don't want one.)

For other background of why this is useful, here is a discussion about the same request for GitHub PRs: https://github.com/orgs/community/discussions/4618

I have a first attempt at an implementation to get a feel for how it would work, but I wanted to get feedback on the idea first before requesting review of that.
@chrisnc chrisnc added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels May 24, 2023
@metbosch
Copy link

In our organization, we will be very happy with that feature. The rebase & fast-forward has several drawbacks if some merge commit was made in the PR commits by any member.

My opinion is that the fast-forward (only) option should replace the current rebase & fast-forward. The rebase part of this option can be achieved by the update (by rebase strategy) button. This button appears in the PR view when the code is not on top of target branch.

@chrisnc
Copy link
Contributor Author

chrisnc commented Jul 20, 2023

I think those two merge strategies would continue to co-exist. The rebase + fast-forward in one click is convenient for projects where that strategy is desired.

@harmathy
Copy link

»Rebase then fast-forward« is already the equivalent of --ff-only. Well in practice it is

git merge --ff-only || git rebase

But what is discussed in https://github.com/orgs/community/discussions/4618 is about support of the equivalent of --ff, which is exactly what I proposed for gitea in #20769.

@chrisnc
Copy link
Contributor Author

chrisnc commented Aug 26, 2023 •
edited
Loading

If you read the description and replies of that GitHub community issue, it's clear that the OP is talking about "fast-forward only" (they do not want merge commits), and the discussion is about the drawbacks of using rebase-and-merge, which is the currently available merge strategy that avoids merge commits on GitHub. The title is misleading. It becomes clear if you replace "--ff" with "fast-forward" in the title. They may not be aware of the fallback behavior of the --ff flag or that it is the default behavior of git merge, because they wrote:

If we have an option to only allow merges of PRs with --ff, we will keep the commit hashes while not having a new merge commit.

Regardless, I think both --ff and --ff-only are valid to support, and one does not obviate the other really. For the way I use git though, --ff-only is what I want, which is why I opened this.

@theoparis
Copy link

I would also like to see this added as this is the only blocker preventing me from switching to gitea over gitlab. Gitlab currently uses a lot more RAM which is unfortunate.

@sebthom
Copy link

sebthom commented Jan 26, 2024 •
edited
Loading

This is one of the top requested features for GitHub. Implementing this would really set Gitea apart. All the reasons why this is extremely useful (like signed commits + linear git history) can be found here https://github.com/orgs/community/discussions/4618

@delvh
Copy link
Member

delvh commented Jan 26, 2024

To summarize what I've seen so far:

  • There seems to be little harm in offering it,
  • it most likely won't require a lot of maintainence
  • it brings some benefits, especially regarding repo security
  • there seems to be interest in using this feature.

As far as I'm concerned, this means the proposal is accepted.
If anyone is willing to open a PR, please go ahead.

@delvh delvh added proposal/accepted We have reviewed the proposal and agree that it should be implemented like that/at all. and removed type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Jan 26, 2024
@delvh
Copy link
Member

delvh commented Jan 26, 2024

Although, on second thought, I do have an implementation question:
This most likely affects two places, right?

  1. The merge box of a PR
  2. The repo settings to enable/disable these merges

Both would need to be added by a proposed PR.
Or did I miss any other change that needs to be done?

chrisnc added a commit to chrisnc/gitea that referenced this issue Jan 27, 2024
@chrisnc
Add a fast-forward-only merge style. (go-gitea#24906)
8314881
chrisnc added a commit to chrisnc/gitea that referenced this issue Jan 27, 2024
@chrisnc
Add a fast-forward-only merge style. (go-gitea#24906)
73b2151
@chrisnc
Copy link
Contributor Author

chrisnc commented Jan 27, 2024

Great! I've rebased the implementation I mentioned in the initial post here and resolved conflicts and made sure it builds, and submitted it as a draft (#28954). I think the two items you mention are the main changes needed for this, though I could be forgetting something myself. I'll continue discussion on the PR.

chrisnc added a commit to chrisnc/gitea that referenced this issue Jan 27, 2024
@chrisnc
Add a fast-forward-only merge style. (go-gitea#24906)
7ede430
@delvh delvh mentioned this issue Jan 27, 2024
Merged
chrisnc added a commit to chrisnc/gitea that referenced this issue Jan 28, 2024
@chrisnc
Add merge style fast-forward-only
c2ef4c9 
With this option, it is possible to require a linear commit history with
the following benefits over the next best option Rebase+fast-forward:
the original commits continue existing, with the original signatures
continuing to stay valid instead of being rewritten, there is no merge
commit, and reverting commits becomes easier.

Closes go-gitea#24906
chrisnc added a commit to chrisnc/gitea that referenced this issue Jan 28, 2024
@chrisnc
Add merge style fast-forward-only
828ceb0 
With this option, it is possible to require a linear commit history with
the following benefits over the next best option Rebase+fast-forward:
the original commits continue existing, with the original signatures
continuing to stay valid instead of being rewritten, there is no merge
commit, and reverting commits becomes easier.

Closes go-gitea#24906
chrisnc added a commit to chrisnc/gitea that referenced this issue Jan 28, 2024
@chrisnc
Add merge style fast-forward-only
f45c6c8 
With this option, it is possible to require a linear commit history with
the following benefits over the next best option Rebase+fast-forward:
the original commits continue existing, with the original signatures
continuing to stay valid instead of being rewritten, there is no merge
commit, and reverting commits becomes easier.

Closes go-gitea#24906
delvh pushed a commit that referenced this issue Feb 12, 2024
@chrisnc
Add merge style fast-forward-only (#28954)
47b5965 
With this option, it is possible to require a linear commit history with
the following benefits over the next best option `Rebase+fast-forward`:
The original commits continue existing, with the original signatures
continuing to stay valid instead of being rewritten, there is no merge
commit, and reverting commits becomes easier.

Closes #24906
silverwind pushed a commit to silverwind/gitea that referenced this issue Feb 15, 2024
@chrisnc @silverwind
Add merge style fast-forward-only (go-gitea#28954)
782a96d 
With this option, it is possible to require a linear commit history with
the following benefits over the next best option `Rebase+fast-forward`:
The original commits continue existing, with the original signatures
continuing to stay valid instead of being rewritten, there is no merge
commit, and reverting commits becomes easier.

Closes go-gitea#24906
silverwind pushed a commit to silverwind/gitea that referenced this issue Feb 20, 2024
@chrisnc @silverwind
Add merge style fast-forward-only (go-gitea#28954)
74f7b6f 
With this option, it is possible to require a linear commit history with
the following benefits over the next best option `Rebase+fast-forward`:
The original commits continue existing, with the original signatures
continuing to stay valid instead of being rewritten, there is no merge
commit, and reverting commits becomes easier.

Closes go-gitea#24906
6543 pushed a commit to 6543-forks/gitea that referenced this issue Feb 26, 2024
@chrisnc @algernon
Add merge style fast-forward-only (go-gitea#28954)
83123b4 
With this option, it is possible to require a linear commit history with
the following benefits over the next best option `Rebase+fast-forward`:
The original commits continue existing, with the original signatures
continuing to stay valid instead of being rewritten, there is no merge
commit, and reverting commits becomes easier.

Closes go-gitea#24906
@github-actions GitHub Actions
Copy link

Automatically locked because of our CONTRIBUTING guidelines

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
proposal/accepted We have reviewed the proposal and agree that it should be implemented like that/at all. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add merge style fast-forward-only chrisnc/gitea
6 participants
@sebthom @harmathy @chrisnc @metbosch @theoparis @delvh