Seems reasonable to me. Thanks!
-D... On Fri, Jan 27, 2017 at 7:02 PM, Casey Stella <ceste...@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 <ceste...@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 <dlyle65...@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 <ceste...@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 <dlyle65...@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 <ceste...@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 > >> > > > > >> > > > >> > > >> > > > > >
