Hi, Cameron, That's generally the case. Thanks for Xinyu to bring this to our attention! +1 to the stated guidelines.
-Yi On Fri, May 17, 2019 at 11:10 AM Cameron Lee <cameronlee...@gmail.com> wrote: > Thanks Xinyu for starting this thread. > I support the guidelines that you mentioned, with a couple clarifications > regarding "PR Review": If a Samza PR is authored by a committer, then > another second committer should provide an approval before that code is > merged, correct? Once the second committer provides approval, then are we > ok with any committer (including the PR author) doing the merge? > > Cameron > > On Thu, May 16, 2019 at 11:30 AM Xinyu Liu <xinyuliu...@gmail.com> wrote: > > > Hi, all, > > > > I've seen different practices around how PRs are contributed, reviewed > and > > merged for Samza open source. I think it's time to bring up our committer > > guide again to make sure we follow exactly the guidelines. It's also an > > opportunity to talk about future improvement to the flow. > > > > *PR Contribution* > > According to our committer guide [1], a JIRA must be created before > > creating the PR, unless the PR is trivial typo or doc fixes. The PR needs > > to have the JIRA ticket name in the following format: > > > > *SAMZA-<JiraNumber> : <JiraTitle>* > > > > As an example: > > SAMZA-2168: Remove redundant SystemAdmin creation in ApplicationMaster > [2]. > > > > *PR Review* > > As discussed before, a Samza PR requires an approval from a committer > > before merging. Contributors are welcome to review the code, but a final > > "LGTM" from a committer is a MUST. > > > > *PR merge* > > As we now use the simple merge flow in github to merge a PR, I think we > > should mostly squash the commits for merging.Otherwise it's hard to roll > > back changes and it generally generates a lot of noise in the commit > > history. > > > > Any further suggestions are highly appreciated. > > > > Thanks, > > Xinyu > > > > [1] > > https://cwiki.apache.org/confluence/display/SAMZA/Contributor%27s+Corner > > [2] https://github.com/apache/samza/pull/1001 > > >
