samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yi Pan <nickpa...@gmail.com>
Subject Re: [DISCUSS] Hygene for merging PRs
Date Mon, 20 May 2019 05:41:38 GMT
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 <cameronlee314@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.us@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
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message