metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Otto Fowler <ottobackwa...@gmail.com>
Subject Re: [DISCUSS] Contributions with multiple authors and requisite modifications to the development guide
Date Mon, 30 Jan 2017 12:00:35 GMT
Maybe using something like this?
https://developer.github.com/v3/pulls/#list-commits-on-a-pull-request


On January 28, 2017 at 09:52:16, Otto Fowler (ottobackwards@gmail.com)
wrote:

Would it be possible to get all the authors out of the pr before squashing
and change out commit messages?  Maybe using an extended version of nick's
scripts?

On January 27, 2017 at 19:02:54, 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