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

Document how maintainers commit things into sosreport #1856

Closed
BryanQuigley opened this issue Nov 12, 2019 · 6 comments
Closed

Document how maintainers commit things into sosreport #1856

BryanQuigley opened this issue Nov 12, 2019 · 6 comments

Comments

@BryanQuigley
Copy link
Contributor

I'm guessing no one else uses the GH interface itself?

I just used Squash and merge to #1850 and added my name as a sign off. Is this effectively what is being down on the command line? Is there a preference? (I'd like to disable Merge commits at least on the GH interface).

Thanks!

@bmr-cymru
Copy link
Member

bmr-cymru commented Nov 12, 2019

Speaking generally it's best not to use squash merges: they aren't as bad as merge commits (they are actually acceptable for single commit branches), but where there are multiple commits on a branch they flatten out the change history.

Typically we merge at the command line - in the past it was not possible to disable merge commits in the web interface, so we have tended to avoid it.

There should never be any merge commits in sos master - branches should be rebased and then merged (this is actually mentioned in the template/guidelines, but the person doing the merge will often do a local rebase before the final git merge command is issued).

@BryanQuigley
Copy link
Contributor Author

I've gone ahead and disabled Merge and "Squash and Merge", only leaving Rebase merging.

The only negative I see of doing a Rebase merge on the GH interface, would be you can't add the additional sign-off line. Thoughts on using that? Apparently I can't disable all 3.

Is the command line basically the same GH shows, but just adding signoff to merge? (not saying that PR is ready, just trying to map it out for me):
git checkout -b slashdd-snapstuffs master
git pull https://github.com/slashdd/sos.git snapstuffs

git checkout master
git merge --signoff --no-ff slashdd-snapstuffs
git push origin master

@bmr-cymru
Copy link
Member

I would still recommend using the command line - I've never even attempted to resolve a conflict in the GitHub webui (is it possible?) but they are not infrequent with sos pull requests and I having the ability to run pycodestyle and the test suite locally on the results before publishing is valuable.

My normal workflow is roughly like this:

$ git checkout -b bmr-merge
$ sospull TurboTurtle:somebranch
$ git rebase master
$ git commit -s --amend # add Signed-off-by and Resolves/Closes tags
$ git checkout master
$ git merge bmr-merge
$ git branch -d bmr-merge

Lather/rinse/repeat as necessary. Depending on conflicts in the branch sometimes it's easier to git pull --rebase (otherwise you can end up resolving the conflict once when you first pull it onto your tree, and a second time when you rebase over master...).

The sospull script is just a shell function:

sospull () { git pull https://github.com/$(echo $@|sed 's/:/\/sos /'); }

You can also set up new git "remotes" for repos you frequently interact with - then you can checkout branches from them directly (creating a local tracking branch) and push/pull between them with the normal git commands. I only tend to do that if I'm often working with the same repo or if I need to update someone else's branch in a pull request.

If I understand --no-ff right then we don't want to use that - it forces creation of a merge commit even if the merge can be made as a fast-forward.

@BryanQuigley
Copy link
Contributor Author

Thanks for those details! Will give that a try!

Rebase on Github only works if a full rebase is possible without merge conflicts (and I can set it up so it requires a clean from CI). You can also require a specific number of acks needed before it can be committed.

Apparently they did add web UI editor to resolve conflicts - but I wouldn't want to use that, see #1419 for an current example of where GH wouldn't allow a rebase merge to go ahead.

@BryanQuigley
Copy link
Contributor Author

Documented here: https://github.com/sosreport/sos/wiki/Maintainers-how-to-merge-PR

I think requiring only clean Rebases on the web interface is likely fine too, but I'm happy to just do it that way.

Thanks again!

@slashdd
Copy link

slashdd commented Nov 19, 2019

@bmr-cymru @pmoravec @TurboTurtle

I don't know if it's the appropriate place for that, but what would you guys think if we do for instance a quarter meeting across vendors ? Redhat/Canonical/... and possibly any other interested vendors.

@BryanQuigley and I (both representing Canonical) would be very interested to have such meeting with you guys.

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

3 participants