These are good suggestions. I would like to suggest some changes. I don't think we can have the guidelines lead with automatic "Rebase and merge", yet at the same time demand the highest quality from merged code. I would suggest that the guideline begin with the emphasis that the committer ensure that the rebased code is still good to go before merge and not on the actual github method. The committer can assess the best method based on the situation. It could be a local rebase on their machine with verification and merge, automatic rebase and merge from github or going back to the contributor if needed. We can explicitly call these out as well.
It will be good that the committer property in the commit no longer be trivial, which has been a pet peeve of mine and will contain the committer information. We should ensure that it is set in all cases. For example, if the change before rebase, is already on top of the latest commit on master, we should still rebase with a --no-ff option (if rebasing manually) or equivalent to get the committer information. We will need to verify that github "Rebase and merge" does this as well. Lastly, along with what is being said, it is also important the way things are said. I think we can still convey the importance and seriousness of following the guidelines without them sounding like threats and warnings. That probably was not the intention but I got that sense from a couple of the points. We wouldn't want to turn off folks who are looking to get involved in the project or grow into a committer. Thanks On Sun, Jul 16, 2017 at 12:18 PM, Vlad Rozov <v.ro...@datatorrent.com> wrote: > Committers, > > As you may know Apex ASF git repos were moved to the new gitbox.apache.org > server and as the result of the move committers are granted write access to > mirror repos on Github. This allows us to slightly change guidelines for > committers and straight line PR review and merge process. Below is my > proposal, please review and comment on it: > > 1. All commits must be pushed to github repository, push to gitbox must be > avoided. > 2. PRs may be merged using gitbox UI. Usage of "Create merge commit" > option must be avoided. > 3. Before PR can be merged, all commits must be squashed, there should be > only one commit associated with a single JIRA (no change from prior > policy). It is responsibility of a contributor to squash commits (not a > committer). > 4. A rebase by a contributor was required to avoid creating an empty merge > commit, it is not a requirement now (see below) as we can rely on github > "Rebase and merge" option to avoid empty merge commits. The option provides > an additional benefit as it properly records the committer. > 5. A committer is fully responsible that the committed code improves > quality of the product. Such things as broken builds (compile, license > check, binary files, checkstyle, unit test) after PR is merged is not > acceptable and may bring questions regarding committer rights. In cases > where PR merge results in a broken build, the committer and the contributor > should work together to fix it asap. > 6. A committer may always request a contributor to rebase his/her changes > against the latest upstream/master, but I would suggest that we do not > abuse such requests. In many cases where changes are limited to files or > methods that are not modified in the upstream, such request is not > necessary. I'd delegate that to a committer to decide when he/she is > comfortable not to require a rebase. Note that in most cases, a committer > is a developer who is mostly familiar with the functionality/code being > modified and should be aware of any changes in the upstream. It is also > possible to request Apache Jenkins to retest the PR if in doubts (Jenkins > PR builds are configured to merge PR branch with the master). > > Thank you, > > Vlad >
