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

[BEAM-9833] Fix .asf.yaml issues, sort labels and disable rebase button #11613

Merged
merged 2 commits into from May 14, 2020

Conversation

iemejia
Copy link
Member

@iemejia iemejia commented May 5, 2020

Issues reported by yamllint and some minor fixes. Also set merge button as the only strategy because we don't want to encourage (o even make possible the other two).

R: @pabloem
CC: @ibzib

@iemejia iemejia requested a review from pabloem May 5, 2020 18:16
@robertwb
Copy link
Contributor

robertwb commented May 5, 2020

I thought consensus was that we did want to encourage squash for those PRs with a huge pile of fixup commits (and otherwise no semantically meaningful commits).

@iemejia
Copy link
Member Author

iemejia commented May 5, 2020

@robertwb Agree, a consensus that nobody respects :). In the case of this PR I set into the merge approach because we have only three options and the other two (squash and rebase) do not create the additional merge commit required by the Beam committer guide.

@pabloem
Copy link
Member

pabloem commented May 5, 2020

I think squash is used by many, and it facilitates receiving contributions without the extra round trip to the contributors. I think we need to discuss more before removing it. Wdyt @iemejia?

@iemejia
Copy link
Member Author

iemejia commented May 5, 2020

I am ok with squashing if it creates extra commits, but it does not seem to be the case or does it create the additional extra merge commit? I just want to ensure we follow the rules.

Now if the goal is to change the rules maybe we should move the discussion to the ML.
Pinging @kennknowles who created our default policy (extra Merge commit) to see what he thinks.

@robertwb
Copy link
Contributor

OK, the rest of this change looks good. Let's get this in, disabling rebase but keeping the others, and taking squash+merge to the list as a separate discussion (and PR if need be).

@iemejia iemejia changed the title [BEAM-9833] Fix yaml issues, sort labels and merge button as only strategy [BEAM-9833] Fix .asf.yaml issues, sort labels and disable rebase button May 14, 2020
@iemejia
Copy link
Member Author

iemejia commented May 14, 2020

Done the changes suggested by @robertwb let only rebase disabled. Can someone PTAL. Thanks

@pabloem
Copy link
Member

pabloem commented May 14, 2020

LGTM!

@pabloem pabloem merged commit 8d13b4f into apache:master May 14, 2020
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

Successfully merging this pull request may close these issues.

None yet

3 participants