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

Enable auto squashing of commits in GitHub #132

Closed
gvbalaji opened this issue Apr 24, 2023 · 5 comments · Fixed by nephio-project/test-infra#26
Closed

Enable auto squashing of commits in GitHub #132

gvbalaji opened this issue Apr 24, 2023 · 5 comments · Fixed by nephio-project/test-infra#26
Assignees
Labels
area/process-mgmt SIG Release Process Management sig/release
Milestone

Comments

@gvbalaji
Copy link

A PR could potentially contain multiple commts. Squashing all the commits before merging could maintain a cleaner history of code changes.

@SimonTheLeg has shown how this can be done automatically in GitHub and how PR title and description could be used as final commit messages. It also maintains the names of all authors.

Once we get a wider buy in for this , we can implement it for all nephio repos.

@gvbalaji gvbalaji added sig/release area/process-mgmt SIG Release Process Management labels Apr 24, 2023
@gvbalaji gvbalaji added this to the sprint3 milestone Apr 24, 2023
@johnbelamaric
Copy link
Member

Yes please!

Multiple commits are super useful during review time ("show changes since your last review" and even better, if people organize the commits logically). But squashing would be best for the final merge.

@gvbalaji
Copy link
Author

From @SimonTheLeg

  1. What is possible on Github?
    These are the options in Github:

https://docs.google.com/document/d/1SW4acc0950QdDNEvmeHArfNsgKPeY_EIwHBqGBky2CY/edit#bookmark=kix.hcx7q6l4m9lj

  1. What is possible in prow?
    Within tide (component in prow), we can set it up to use “squash” as the merge_method (refer to https://docs.prow.k8s.io/docs/components/core/tide/config/)
    Apart from this, prow does not have any additional capabilities for squash merging. Instead it simply calls the GitHub-API, which then does what is configured in the repo.

  2. How do we handle a PR which has commits from multiple authors?
    Github handles this automatically by adding a Co-authored by xyz line to the commit message. This is then displayed in the UI and persisted in git-history:

(example taken from kubermatic/kubermatic@022ca1e)

@radoslawc
Copy link
Contributor

We can have both, actually all three options really (merge, squash, rebase). Tide has configuration option to have designated labels make it switch merging type. If you label PR with (label name is configurable) let's say 'tide/merge-method-squash' it will squash commits and and github automatically preserves authors as coauthors and commit message is concatenated from included commits.

@radoslawc
Copy link
Contributor

After some thinking, let's make squash default method.

@gvbalaji
Copy link
Author

gvbalaji commented May 3, 2023

Thanks @radoslawc . I assume this is done now. Does it use PR message and description as commit messages?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/process-mgmt SIG Release Process Management sig/release
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants