-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
add Merge Strategy section to README #1284
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just check the typo as the last line: Multiple commits in singl PR make sense in certain cases (e.g. branch backports).
@vorburger should we not update the github PR template then? which says |
Fixed, using Edit on GitHub UI. Which makes this PR itself now have 2 commits, and I'm too lazy to pull it locally and squash, which makes this an example of a PR that whichever committers merges this should squash... as this documents. 😸 PS: @awasum et al just just use GitHub Suggestions to propose changes directly! (Instead of only "commenting".) |
looks like we are about to have 3 commits as an example now :P |
great point / feedback, I didn't think of that! Thanks - done now.
@thesmallstar yeah, you made me pull it after all (or perhaps I'm too stupid to know how to edit multiple files on GitHub's Web UI). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@vorburger this failed due to connection error (but I think you can merge this anyways)... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Petri Tuomola <[email protected]>
Co-authored-by: Petri Tuomola <[email protected]>
Co-authored-by: Petri Tuomola <[email protected]>
Co-authored-by: Petri Tuomola <[email protected]>
@ptuomola @xurror @awasum @avikganguly01 @thesmallstar @percyashu @fynmanoj @vidakovic @conradsp @Grandolf49 how about (something like) this to document the recommended Merge Strategy for this project? Your review feedback, if any, most welcome, of course.