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

How should the Source track address merge strategies? #1040

Open
kpk47 opened this issue Mar 28, 2024 · 4 comments
Open

How should the Source track address merge strategies? #1040

kpk47 opened this issue Mar 28, 2024 · 4 comments

Comments

@kpk47
Copy link
Contributor

kpk47 commented Mar 28, 2024

          Some communities are very adamant about _never_ squashing commits, instead using commits to describe logical bits of change and then just merging them all in to preserve the blame. While this might introduce some risk (i.e. by creating reachable malicious commits), that risk can be mitigated if all commits are reviewed during the process. 

While squash+merge might be a best practice from a security perspective, I am not sure whether we can be overly prescriptive of it as a requirement for this track/level.

Originally posted by @arewm in #1037 (comment)

The Source track currently requires squash+merge, but @arewm points out that not all communities are open to that merge strategy. Should we maintain the requirement for squash+merge and evangelize that strategy's benefits, or should we accommodate others? If the latter, then which ones?

@david-a-wheeler
Copy link
Member

I suggest dropping the requirement to mandate squash+merge. It's controversial & you can make arguments in both directions. For example, squash+merge removes the history that lets you see what problems were found & fixed by a review; indeed, it mostly hides the review. I don't think requiring squash+merge is a hill worth dying on :-).

@adityasaky
Copy link
Contributor

+1, I think requiring squash merge is not the best idea. IMO, there's value in preserving not just the history of the feature branch but also the signatures on the commits that are merged in.

@MarkLodato
Copy link
Member

+1, it should not require squash merge.

Instead, I think what we want is that it's possible to know which commits meet the requirements (retention, attribution, etc) and which do not. For example, in a GitHub model where reviewed only look at the merge commit but not the intermediate garbage commit, there would be some way for the verifier to know that only the merge commits are "good" but that the intermediate commits have no claims. This could be by a convention that it's always "first parent history" (using git terminology), or that the merge commits are signed, or something else.

@joshuagl
Copy link
Member

I'm with the other commenters, trying to mandate merge strategy will make the source track a no-go for many (open source projects and not).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

No branches or pull requests

5 participants