metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Lyle <dlyle65...@gmail.com>
Subject Re: [DISCUSS] Contributions with multiple authors and requisite modifications to the development guide
Date Sat, 28 Jan 2017 12:28:28 GMT
Seems reasonable to me.

Thanks!

-D...

On Fri, Jan 27, 2017 at 7:02 PM, Casey Stella <cestella@gmail.com> wrote:

> Having thought about it a minute longer, maybe you're right, Dave.  I like
> this.
>
> I'd add one sentence more:
> If the contribution does not have a single author, please ask that the
> contribution be squashed by the author into the minimum number of commits
> required to preserve correct attribution each following the same standard
> naming convention of: JIRA Description (e.g. METRON-123: Foo all the Bars).
> The committer should not squash commits in this instance so that ownership
> is maintained.
>
> On Fri, Jan 27, 2017 at 6:55 PM, Casey Stella <cestella@gmail.com> wrote:
>
> > Yeah, this is sort of the reason I was going for multiple PRs rather than
> > having contributors do their own squashing and commits.  I recognize that
> > multiple PRs are difficult to review.
> >
> > That being said, I don't know that the actual commits are required to
> have
> > all of a person's commit.  I imagined the process being:
> > * The contributors squashes their commits owned by one of them
> > * The others add an empty commit for each of the contributors
> > * The committer then just does a straight pull rather than a squashed
> pull.
> >
> > My understanding is that we need to keep a record of who contributed to
> > what JIRA not that the actual commits reflect the actual code contributed
> > (which can be quite difficult to disentangle, I imagine).  Please correct
> > me if I'm wrong here, though.
> >
> > I'm being prescriptive because I'm worried that "do the right thing" will
> > result in us not doing the right thing, sadly.
> >
> > Thoughts?
> >
> > On Fri, Jan 27, 2017 at 6:03 PM, David Lyle <dlyle65535@gmail.com>
> wrote:
> >
> >> We may be trying to be too prescriptive, it won't always be possible to
> >> have a single commit per author (interleaved changes).
> >>
> >> Maybe
> >>
> >> If the contribution does not have a single author, please ask that the
> >> contribution be squashed by the author into the minimum number of
> commits
> >> required to preserve correct attribution each following the same
> standard
> >> naming convention of: JIRA Description (e.g. METRON-123: Foo all the
> >> Bars).
> >>
> >> ?
> >>
> >> I really don't know- the real message is, do the right thing to make
> sure
> >> everyone's contribution is duly recorded. So the commit log would be
> >> something like
> >> Author: 1 METRON-123 Foo all the bars - foo'd some bars
> >> Author: 2 METRON-123 Foo all the bars - helped for a better foo
> >> Author: 1 METRON-123 Fool all the bars - But, there was this little deal
> >>
> >> I think it's hard to cover all the cases, so maybe a statement of the
> >> principle we're trying to maintain?
> >>
> >> -D...
> >>
> >>
> >> On Fri, Jan 27, 2017 at 5:50 PM, Casey Stella <cestella@gmail.com>
> wrote:
> >>
> >> > So, then the wording would be:
> >> >
> >> > If the contribution does not have a single author, please ask that the
> >> > contribution be squashed by the author into one commit per author
> >> > each following the same standard naming convention of: JIRA
> >> > Description (e.g. METRON-123: Foo all the Bars).
> >> >
> >> > Fair or did I get it wrong?
> >> >
> >> > On Fri, Jan 27, 2017 at 5:47 PM, David Lyle <dlyle65535@gmail.com>
> >> wrote:
> >> >
> >> > > The bolded didn't come through, but I'm guessing it's here:
> >> > >
> >> > > *If the contribution does not have a single author, please ask that
> >> the
> >> > > contribution be separated into multiple separate pull requests (one
> >> per
> >> > > author with their contributions) that may have (non-cyclical)
> >> > dependencies,
> >> > > each following the same standard naming convention of JIRA: JIRA
> >> > > Description (e.g. METRON-123: Foo all the Bars).*
> >> > >
> >> > > I'd recommend against multiple PRs. Creates all kinds of
> difficulties
> >> > > reviewing and testing. The changeset should be reviewed and tested
> as
> >> a
> >> > > single unit. If there is more than one author, simply don't squash.
> >> > >
> >> > > -D...
> >> > >
> >> > >
> >> > > On Fri, Jan 27, 2017 at 5:44 PM, Casey Stella <cestella@gmail.com>
> >> > wrote:
> >> > >
> >> > > > It is important for every contribution to attribute the author,
> our
> >> git
> >> > > log
> >> > > > is a legal papertrail for attribution.  As a consequence of that,
> I
> >> > > > recommend the Development Guidelines section 1.2 be amended to
the
> >> > > > following (bolded change):
> >> > > >
> >> > > > Everyone is encouraged to review open pull requests. We only
ask
> >> that
> >> > you
> >> > > > try and think carefully, ask questions and are excellent to one
> >> > another.
> >> > > > Code review is our opportunity to share knowledge, design ideas
> and
> >> > make
> >> > > > friends.  The instructions on how to checkout a patch for review
> >> can be
> >> > > > found here <https://github.com/nickwallen/metron-commit-stuff>.
> >> > > >
> >> > > > When reviewing a patch try to keep each of these concepts in
mind:
> >> > > >
> >> > > >
> >> > > >
> >> > > >    - Is the proposed change being made in the correct place?
Is
> it a
> >> > fix
> >> > > in
> >> > > >    a backend when it should be in the primitives?  In Kafka vs
> >> Storm?
> >> > > >    - What is the change being proposed?  Is it based on Community
> >> > > >    recognized issues?
> >> > > >    - Do we want this feature or is the bug they’re fixing really
a
> >> bug?
> >> > > >    - Does the change do what the author claims?
> >> > > >    - Are there sufficient tests?
> >> > > >    - Has it been documented?
> >> > > >    - Will this change introduce new bugs?
> >> > > >    - *Does this contribution have a single author?*
> >> > > >
> >> > > >
> >> > > > *If the contribution does not have a single author, please ask
> that
> >> the
> >> > > > contribution be separated into multiple separate pull requests
> (one
> >> per
> >> > > > author with their contributions) that may have (non-cyclical)
> >> > > dependencies,
> >> > > > each following the same standard naming convention of JIRA: JIRA
> >> > > > Description (e.g. METRON-123: Foo all the Bars).*
> >> > > >
> >> > > > Also, please review if the submitter correctly flagged impacts
to
> >> the
> >> > > > following (if exist):
> >> > > >
> >> > > >    - System Configuration Changes
> >> > > >       - Metron Configuration
> >> > > >       - Metron Component Configuration (sensors, etc)
> >> > > >       - Tech Stack Configuration (Storm, Hbase, etc)
> >> > > >    - Environmental Changes
> >> > > >       - Helper Shell Scripts
> >> > > >       - RPM Packaging
> >> > > >       - Ansible, Ambari, AWS, Docker
> >> > > >    - Documentation Impacts
> >> > > >       - Changes to Wiki Documentation
> >> > > >       - Revisions in Tutorials
> >> > > >       - Developer Guide
> >> > > >       - Expansions in readme's
> >> > > >    - Changes to System Interfaces
> >> > > >       - Stellar Shell
> >> > > >       - REST APIs
> >> > > >       - Etc...
> >> > > >
> >> > > >
> >> > > > Thoughts?  Better way to handle this scenario while still
> >> maintaining
> >> > > > unambiguous authorship?
> >> > > >
> >> > > > Casey
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>

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