Oh, one more, I propose the following addition to 2.5:
>
> JIRAs will have a description of how to exercise the functionality in a
> step-by-step manner on a Quickdev vagrant instance to aid review and
> validation.


When Mike, Otto and I moved the system to the current version of Storm, we
needed a broader smoke test than just running data through that exercised a
variety of the features. We pulled those smoke tests from the various
discussions in the JIRAs.



On Wed, Dec 21, 2016 at 9:38 AM, Casey Stella <[email protected]> wrote:

> We have been having a lively discussion on METRON-590 (see
> https://github.com/apache/incubator-metron/pull/395) around creating
> multiple abstractions to do the same (or very nearly the same) thing.
>
> I'd like to propose an addition to section 2.3 which reads:
>
>> Contributions which provide abstractions which are either very similar to
>> or a subset of existing abstractions should use and extend existing
>> abstractions rather than provide competing abstractions unless engineering
>> exigencies (e.g. performance ) make such an operation impossible without
>> compromising core functionality of the platform.
>
>
> I'd like to suggest the following anecdote from the early years of the
> codebase to justify the above:
>
> Stellar started as a predicate language only for threat triage rules. As
> such, when the task of creating Field Transformations came to me, I needed
> something like Stellar except I needed it to return arbitrary objects,
> rather than just booleans. In my infinite wisdom, I chose to fork the
> language, create a second, more specific DSL for field transformations,
> thereby creating "Metron Query Language" and "Metron Transformation
> Language."
>
> I felt a nagging feeling at the time that I should just expand the query
> language, but I convinced myself that it would require too much testing and
> it would be a change that was too broad in scope. It took 3 months for me
> to get around to unifying those languages and if we had more people using
> it, it would have been an absolute nightmare.
>
> On Wed, Dec 21, 2016 at 9:31 AM, Casey Stella <[email protected]> wrote:
>
>> Yeah, I +1 the notion of thorough automated tests.
>>
>> On Tue, Dec 20, 2016 at 4:36 PM, Matt Foley <[email protected]> wrote:
>>
>>> Hard to mark diffs in text-only mode :-)  My proposed change is:
>>>
>>> >> All merged patches will be reviewed with the expectation that
>>> thorough automated tests shall be provided and are consistent with …
>>>
>>>                                    ^^^^^^^^
>>>  ^^^^^^^^^^^^^^
>>> Added word “thorough” and changed “exist” to “shall be provided”.
>>> Thanks,
>>> --Matt
>>>
>>> On 12/20/16, 1:22 PM, "James Sirota" <[email protected]> wrote:
>>>
>>>     Hi Matt, thats already in there. See last bullet point of 2.6
>>>
>>>     20.12.2016, 14:14, "Matt Foley" <[email protected]>:
>>>     > If we aren't going to require 100% test coverage for new code,
>>> then we should at least say "thorough" automated tests, in the last bullet
>>> of 2.6. And it should be a mandate not an assumption:
>>>     >
>>>     > All merged patches will be reviewed with the expectation that
>>> thorough automated tests shall be provided and are consistent with project
>>> testing methodology and practices, and cover the appropriate cases ( see
>>> reviewers guide )
>>>     >
>>>     > IMO,
>>>     > --Matt
>>>     >
>>>     > On 12/20/16, 12:51 PM, "James Sirota" <[email protected]> wrote:
>>>     >
>>>     >     Good feedback. Here is the next iteration that accounts for
>>> your suggestions:
>>>     >     https://cwiki.apache.org/confluence/pages/viewpage.action?p
>>> ageId=61332235
>>>     >
>>>     >     1. How To Contribute
>>>     >     We are always very happy to have contributions, whether for
>>> trivial cleanups, little additions or big new features.
>>>     >     If you don't know Java or Scala you can still contribute to
>>> the project. We strongly value documentation and gladly accept improvements
>>> to the documentation.
>>>     >     1.1 Contributing A Code Change
>>>     >     To submit a change for inclusion, please do the following:
>>>     >     If there is not already a JIRA associated with your pull
>>> request, create it, assign it to yourself, and start progress
>>>     >     If there is a JIRA already created for your change then assign
>>> it to yourself and start progress
>>>     >     If you don't have access to JIRA or can't assign an issue to
>>> yourself, please message [email protected] and someone
>>> will either give you permission or assign a JIRA to you
>>>     >     If you are introducing a completely new feature or API it is a
>>> good idea to start a discussion and get consensus on the basic design
>>> first. Larger changes should be discussed on the dev boards before
>>> submission.
>>>     >     New features and significant bug fixes should be documented in
>>> the JIRA and appropriate architecture diagrams should be attached. Major
>>> features may require a vote.
>>>     >     Note that if the change is related to user-facing protocols /
>>> interface / configs, etc, you need to make the corresponding change on the
>>> documentation as well.
>>>     >     Craft a pull request following the guidelines in Section 2 of
>>> this document
>>>     >     Pull requests should be small to facilitate easier review.
>>> Studies have shown that review quality falls off as patch size grows.
>>> Sometimes this will result in many small PRs to land a single large feature.
>>>     >     People will review and comment on your pull request. It is our
>>> job to follow up on pull requests in a timely fashion.
>>>     >     Once the pull request is merged the person doing the merge
>>> (committer) should manually close the corresponding JIRA.
>>>     >     1.2 Reviewing and merging patches
>>>     >     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.
>>>     >     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?
>>>     >
>>>     >     2. Implementation
>>>     >
>>>     >     2.1 Grammar and style
>>>     >     These are small things that are not caught by the automated
>>> style checkers.
>>>     >     Does a variable need a better name?
>>>     >     Should this be a keyword argument?
>>>     >     In a PR, maintain the existing style of the file.
>>>     >     Don’t combine code changes with lots of edits of whitespace or
>>> comments; it makes code review too difficult. It’s okay to fix an
>>> occasional comment or indenting, but if wholesale comment or whitespace
>>> changes are needed, make them a separate PR.
>>>     >     Use the checkstyle plugin in Maven to verify that your PR
>>> conforms to our style
>>>     >     2.2 Code Style
>>>     >     Follow the Sun Code Conventions outlined here:
>>> http://www.oracle.com/technetwork/java/codeconvtoc-136057.html
>>>     >     except that indents are 2 spaces instead of 4
>>>     >     2.3 Coding Standards
>>>     >     Implementation matches what the documentation says
>>>     >     Logger name is effectively the result of Class.getName()
>>>     >     Class & member access - as restricted as it can be (subject to
>>> testing requirements)
>>>     >     Appropriate NullPointerException and IllegalArgumentException
>>> argument checks
>>>     >     Asserts - verify they should always be true
>>>     >     Look for accidental propagation of exceptions
>>>     >     Look for unanticipated runtime exceptions
>>>     >     Try-finally used as necessary to restore consistent state
>>>     >     Logging levels conform to Log4j levels
>>>     >     Possible deadlocks - look for inconsistent locking order
>>>     >     Race conditions - look for missing or inadequate
>>> synchronization
>>>     >     Consistent synchronization - always locking the same object(s)
>>>     >     Look for synchronization or documentation saying there's no
>>> synchronization
>>>     >     Look for possible performance problems
>>>     >     Look at boundary conditions for problems
>>>     >     Configuration entries are retrieved/set via setter/getter
>>> methods
>>>     >     Implementation details do NOT leak into interfaces
>>>     >     Variables and arguments should be interfaces where possible
>>>     >     If equals is overridden then hashCode is overridden (and vice
>>> versa)
>>>     >     Objects are checked (instanceof) for appropriate type before
>>> casting (use generics if possible)
>>>     >     Public API changes have been publicly discussed
>>>     >     Use of static member variables should be used with caution
>>> especially in Map/reduce tasks due to the JVM reuse feature
>>>     >     2.4 Documentation
>>>     >
>>>     >     Code-Level Documentation
>>>     >     Self-documenting code (variable, method, class) has a clear
>>> semantic name
>>>     >     Accurate, sufficient for developers to code against
>>>     >     Follows standard Javadoc conventions
>>>     >     Loggers and logging levels covered if they do not follow our
>>> conventions (see below)
>>>     >     System properties, configuration options, and resources covered
>>>     >     Illegal arguments are properly documented as appropriate
>>>     >     Package and overview Javadoc are updated as appropriate
>>>     >     Javadoc comments are mandatory for all public APIs
>>>     >     Generate Javadocs for release builds
>>>     >
>>>     >     Feature-level documentation - should be version controlled in
>>> github in README files.
>>>     >     Accurate description of the feature
>>>     >     Sample configuration and deployment options
>>>     >     Sample usage scenarios
>>>     >
>>>     >     High-Level Design documentation - architecture description and
>>> diagrams should be a part of a wiki entry.
>>>     >     Provide diagrams/charts where appropriate. Visuals are always
>>> welcome
>>>     >     Provide purpose of the feature/module and why it exists within
>>> the project
>>>     >     Describe system flows through the feature/module where
>>> appropriate
>>>     >     Describe how the feature/module interfaces with the rest of
>>> the system
>>>     >     Describe appropriate usages scenarios and use cases
>>>     >
>>>     >     Tutorials - system-level tutorials and use cases should also
>>> be kept as wiki entries.
>>>     >     Add to the Metron reference application documentation for each
>>> additional major feature
>>>     >     If appropriate, publish a tutorials blog on the Wiki to
>>> highlight usage scenarios and apply them to the real world use cases
>>>     >     2.5 Tests
>>>     >     Unit tests exist for bug fixes and new features, or a
>>> rationale is given in JIRA for why there is no test
>>>     >      Unit tests do not write any temporary files to /tmp (instead,
>>> the tests should write to the location specified by the test.build.data
>>> system property)
>>>     >
>>>     >     2.6 Merge requirements
>>>     >     Because Metron is so complex, and the implications of getting
>>> it wrong so devastating, Metron has a strict merge policy for committers:
>>>     >     Patches must never be pushed directly to master, all changes
>>> (even the most trivial typo fixes!) must be submitted as a pull request.
>>>     >     A committer may merge their own pull request, but only after a
>>> second reviewer has given it a +1. A qualified reviewer is a Metron
>>> committer or PPMC member.
>>>     >     A non-committer may ask the reviewer to merge their pull
>>> request or alternatively post to the Metron dev board to get another
>>> committer to merge the PR if the reviewer is not available.
>>>     >     There should be at least one independent party besides the
>>> committer that have reviewed the patch before merge.
>>>     >     A patch that breaks tests, or introduces regressions by
>>> changing or removing existing tests should not be merged. Tests must always
>>> be passing on master. This implies that the tests have been run.
>>>     >     All pull request submitters must link to travis-ci
>>>     >     If somehow the tests get into a failing state on master (such
>>> as by a backwards incompatible release of a dependency) no pull requests
>>> may be merged until this is rectified.
>>>     >     All merged patches will be reviewed with the expectation that
>>> automated tests exist and are consistent with project testing methodology
>>> and practices, and cover the appropriate cases ( see reviewers guide )
>>>     >
>>>     >     The purpose of these policies is to minimize the chances we
>>> merge a change that has unintended consequences.
>>>     >
>>>     >     3. JIRA
>>>     >     The Incompatible change flag on the issue's JIRA is set
>>> appropriately for this patch
>>>     >     For incompatible changes, major features/improvements, and
>>> other release notable issues, the Release Note field has a sufficient
>>> comment
>>>     >
>>>     >     20.12.2016, 13:18, "Otto Fowler" <[email protected]>:
>>>     >     > "The purpose of these policies is to minimize the chances we
>>> merge a change
>>>     >     > that jeopardizes has unintended consequences."
>>>     >     >
>>>     >     > remove jeopardizes?
>>>     >     >
>>>     >     > On December 20, 2016 at 13:25:35, James Sirota (
>>> [email protected]) wrote:
>>>     >     >
>>>     >     > I incorporated the changes. This is what the doc looks like
>>> now:
>>>     >     > https://cwiki.apache.org/confluence/pages/viewpage.action?pa
>>> geId=61332235
>>>     >     >
>>>     >     > As an open source project, Metron welcomes contributions of
>>> all forms. The
>>>     >     > sections below will help you get started.
>>>     >     > 1. How To Contribute
>>>     >     > We are always very happy to have contributions, whether for
>>> trivial
>>>     >     > cleanups, little additions or big new features.
>>>     >     > If you don't know Java or Scala you can still contribute to
>>> the project. We
>>>     >     > strongly value documentation and gladly accept improvements
>>> to the
>>>     >     > documentation.
>>>     >     > 1.1 Contributing A Code Change
>>>     >     > To submit a change for inclusion, please do the following:
>>>     >     > If there is not already a JIRA associated with your pull
>>> request, create
>>>     >     > it, assign it to yourself, and start progress
>>>     >     > If there is a JIRA already created for your change then
>>> assign it to
>>>     >     > yourself and start progress
>>>     >     > If you don't have access to JIRA or can't assign an issue to
>>> yourself,
>>>     >     > please message [email protected] and someone
>>> will give you
>>>     >     > permission
>>>     >     > If you are introducing a completely new feature or API it is
>>> a good idea to
>>>     >     > start a discussion and get consensus on the basic design
>>> first. Larger
>>>     >     > changes should be discussed on the dev boards before
>>> submission.
>>>     >     > New features and significant bug fixes should be documented
>>> in the JIRA and
>>>     >     > appropriate architecture diagrams should be attached. Major
>>> features may
>>>     >     > require a vote.
>>>     >     > Note that if the change is related to user-facing protocols
>>> / interface /
>>>     >     > configs, etc, you need to make the corresponding change on
>>> the
>>>     >     > documentation as well.
>>>     >     > Craft a pull request following the guidelines in Section 2
>>> of this document
>>>     >     > Pull requests should be small to facilitate easier review.
>>> Studies have
>>>     >     > shown that review quality falls off as patch size grows.
>>> Sometimes this
>>>     >     > will result in many small PRs to land a single large feature.
>>>     >     > People will review and comment on your pull request. It is
>>> our job to
>>>     >     > follow up on pull requests in a timely fashion.
>>>     >     > Once the pull request is merged, manually close the
>>> corresponding JIRA
>>>     >     > 1.2 Reviewing and merging patches
>>>     >     > 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.
>>>     >     > 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?
>>>     >     >
>>>     >     > 2. Implementation
>>>     >     >
>>>     >     > 2.1 Grammar and style
>>>     >     > These are small things that are not caught by the automated
>>> style checkers.
>>>     >     > Does a variable need a better name?
>>>     >     > Should this be a keyword argument?
>>>     >     > In a PR, maintain the existing style of the file.
>>>     >     > Don’t combine code changes with lots of edits of whitespace
>>> or comments; it
>>>     >     > makes code review too difficult. It’s okay to fix an
>>> occasional comment or
>>>     >     > indenting, but if wholesale comment or whitespace changes
>>> are needed, make
>>>     >     > them a separate PR.
>>>     >     > Use the checkstyle plugin in Maven to verify that your PR
>>> conforms to our
>>>     >     > style
>>>     >     > 2.2 Code Style
>>>     >     > Follow the Sun Code Conventions outlined here:
>>>     >     > http://www.oracle.com/technetwork/java/codeconvtoc-136057.ht
>>> ml
>>>     >     > except that indents are 2 spaces instead of 4
>>>     >     > 2.3 Coding Standards
>>>     >     > Implementation matches what the documentation says
>>>     >     > Logger name is effectively the result of Class.getName()
>>>     >     > Class & member access - as restricted as it can be (subject
>>> to testing
>>>     >     > requirements)
>>>     >     > Appropriate NullPointerException and
>>> IllegalArgumentException argument
>>>     >     > checks
>>>     >     > Asserts - verify they should always be true
>>>     >     > Look for accidental propagation of exceptions
>>>     >     > Look for unanticipated runtime exceptions
>>>     >     > Try-finally used as necessary to restore consistent state
>>>     >     > Logging levels conform to Log4j levels
>>>     >     > Possible deadlocks - look for inconsistent locking order
>>>     >     > Race conditions - look for missing or inadequate
>>> synchronization
>>>     >     > Consistent synchronization - always locking the same
>>> object(s)
>>>     >     > Look for synchronization or documentation saying there's no
>>> synchronization
>>>     >     > Look for possible performance problems
>>>     >     > Look at boundary conditions for problems
>>>     >     > Configuration entries are retrieved/set via setter/getter
>>> methods
>>>     >     > Implementation details do NOT leak into interfaces
>>>     >     > Variables and arguments should be interfaces where possible
>>>     >     > If equals is overridden then hashCode is overridden (and
>>> vice versa)
>>>     >     > Objects are checked (instanceof) for appropriate type before
>>> casting (use
>>>     >     > generics if possible)
>>>     >     > Public API changes have been publicly discussed
>>>     >     > Use of static member variables should be used with caution
>>> especially in
>>>     >     > Map/reduce tasks due to the JVM reuse feature
>>>     >     > 2.4 Documentation
>>>     >     >
>>>     >     > Code-Level Documentation
>>>     >     > Self-documenting code (variable, method, class) has a clear
>>> semantic name
>>>     >     > Accurate, sufficient for developers to code against
>>>     >     > Follows standard Javadoc conventions
>>>     >     > Loggers and logging levels covered if they do not follow our
>>> conventions
>>>     >     > (see below)
>>>     >     > System properties, configuration options, and resources
>>> covered
>>>     >     > Illegal arguments are properly documented as appropriate
>>>     >     > Package and overview Javadoc are updated as appropriate
>>>     >     > Javadoc comments are mandatory for all public APIs
>>>     >     > Generate Javadocs for release builds
>>>     >     >
>>>     >     > Feature-level documentation - should be version controlled
>>> in github in
>>>     >     > README files.
>>>     >     > Accurate description of the feature
>>>     >     > Sample configuration and deployment options
>>>     >     > Sample usage scenarios
>>>     >     >
>>>     >     > High-Level Design documentation - architecture description
>>> and diagrams
>>>     >     > should be a part of a wiki entry.
>>>     >     > Provide diagrams/charts where appropriate. Visuals are
>>> always welcome
>>>     >     > Provide purpose of the feature/module and why it exists
>>> within the project
>>>     >     > Describe system flows through the feature/module where
>>> appropriate
>>>     >     > Describe how the feature/module interfaces with the rest of
>>> the system
>>>     >     > Describe appropriate usages scenarios and use cases
>>>     >     >
>>>     >     > Tutorials - system-level tutorials and use cases should also
>>> be kept as
>>>     >     > wiki entries.
>>>     >     > Add to the Metron reference application documentation for
>>> each additional
>>>     >     > major feature
>>>     >     > If appropriate, publish a tutorials blog on the Wiki to
>>> highlight usage
>>>     >     > scenarios and apply them to the real world use cases
>>>     >     > 2.5 Tests
>>>     >     > Unit tests exist for bug fixes and new features, or a
>>> rationale is given in
>>>     >     > JIRA for why there is no test
>>>     >     > Unit tests do not write any temporary files to /tmp
>>> (instead, the tests
>>>     >     > should write to the location specified by the
>>> test.build.data system
>>>     >     > property)
>>>     >     >
>>>     >     > 2.6 Merge requirements
>>>     >     > Because Metron is so complex, and the implications of
>>> getting it wrong so
>>>     >     > devastating, Metron has a strict merge policy for committers:
>>>     >     > Patches must never be pushed directly to master, all changes
>>> (even the most
>>>     >     > trivial typo fixes!) must be submitted as a pull request.
>>>     >     > A committer may merge their own pull request, but only after
>>> a second
>>>     >     > reviewer has given it a +1. A qualified reviewer is a Metron
>>> committer or
>>>     >     > PPMC member.
>>>     >     > A non-committer may ask the reviewer to merge their pull
>>> request or
>>>     >     > alternatively post to the Metron dev board to get another
>>> committer to
>>>     >     > merge the PR if the reviewer is not available.
>>>     >     > There should be at least one independent party besides the
>>> committer that
>>>     >     > have reviewed the patch before merge.
>>>     >     > A patch that breaks tests, or introduces regressions by
>>> changing or
>>>     >     > removing existing tests should not be merged. Tests must
>>> always be passing
>>>     >     > on master. This implies that the tests have been run.
>>>     >     > All pull request submitters must link to travis-ci
>>>     >     > If somehow the tests get into a failing state on master
>>> (such as by a
>>>     >     > backwards incompatible release of a dependency) no pull
>>> requests may be
>>>     >     > merged until this is rectified.
>>>     >     > All merged patches will be reviewed with the expectation
>>> that automated
>>>     >     > tests exist and are consistent with project testing
>>> methodology and
>>>     >     > practices, and cover the appropriate cases ( see reviewers
>>> guide )
>>>     >     >
>>>     >     > The purpose of these policies is to minimize the chances we
>>> merge a change
>>>     >     > that jeopardizes has unintended consequences.
>>>     >     >
>>>     >     > 3. JIRA
>>>     >     > The Incompatible change flag on the issue's JIRA is set
>>> appropriately for
>>>     >     > this patch
>>>     >     > For incompatible changes, major features/improvements, and
>>> other release
>>>     >     > notable issues, the Release Note field has a sufficient
>>> comment
>>>     >     >
>>>     >     > 20.12.2016, 09:42, "[email protected]" <[email protected]>:
>>>     >     >> I don't have enough experience on the Java/Javadoc side to
>>> make a
>>>     >     >
>>>     >     > specific
>>>     >     >> suggestion, but with other languages I've used Sphinx and
>>> Doxygen with
>>>     >     >> great results.
>>>     >     >>
>>>     >     >> Jon
>>>     >     >>
>>>     >     >> On Tue, Dec 20, 2016 at 11:29 AM Michael Miklavcic <
>>>     >     >> [email protected]> wrote:
>>>     >     >>
>>>     >     >>> Were you thinking javadoc or something more? I wouldn't
>>> mind seeing us
>>>     >     >>> produce a javadoc site, if we aren't already doing so.
>>>     >     >>>
>>>     >     >>> On Dec 20, 2016 9:25 AM, "[email protected]" <
>>> [email protected]> wrote:
>>>     >     >>>
>>>     >     >>> > Regarding documentation - while I'm not a huge fan of
>>> that approach
>>>     >     >
>>>     >     > (I
>>>     >     >>> > would prefer to see documentation generated from the
>>> code), I think
>>>     >     >
>>>     >     > it
>>>     >     >>> > could work in the short term. Having that outlined both
>>> in the coding
>>>     >     >>> > guidelines and on the wiki would be important.
>>>     >     >>> >
>>>     >     >>> > I agree with the comments about author != committer, and
>>> 100% code
>>>     >     >>> > coverage.
>>>     >     >>> >
>>>     >     >>> > Jon
>>>     >     >>> >
>>>     >     >>> > On Tue, Dec 20, 2016 at 11:10 AM James Sirota <
>>> [email protected]>
>>>     >     >>> wrote:
>>>     >     >>> >
>>>     >     >>> > > In my view the lower-level documentation that should
>>> be source
>>>     >     >>> controlled
>>>     >     >>> > > with the code belongs on github and then use case
>>> documentation and
>>>     >     >>> > > top-level architecture diagrams belong on the wiki.
>>> What do you
>>>     >     >
>>>     >     > think?
>>>     >     >>> > >
>>>     >     >>> > > I think if the author is not a committer and can't
>>> merge then the
>>>     >     >>> > reviewer
>>>     >     >>> > > should probably merge or the PR originator should ping
>>> the dev
>>>     >     >
>>>     >     > board to
>>>     >     >>> > get
>>>     >     >>> > > someone to merge the PR in. Does that seem reasonable
>>> to everyone?
>>>     >     >>> > >
>>>     >     >>> > > 18.12.2016, 13:10, "Kyle Richardson" <
>>> [email protected]>:
>>>     >     >>> > > > Couple of questions/comments:
>>>     >     >>> > > >
>>>     >     >>> > > > In 2.4, we talk about Javadoc and code comments but
>>> not too much
>>>     >     >>> about
>>>     >     >>> > > the
>>>     >     >>> > > > user documentation. Should we, possibly in a section
>>> 4, give some
>>>     >     >>> > > > recommendations on what should go into the README
>>> files versus on
>>>     >     >
>>>     >     > the
>>>     >     >>> > > wiki?
>>>     >     >>> > > > This could also help the reviewer know if the change
>>> is
>>>     >     >
>>>     >     > documented
>>>     >     >>> > > > sufficiently.
>>>     >     >>> > > >
>>>     >     >>> > > > In 2.6, we say that 1 qualified reviewer (Apache
>>> committer or
>>>     >     >
>>>     >     > PPMC
>>>     >     >>> > > member)
>>>     >     >>> > > > other than the author of the PR must have given it a
>>> +1. In the
>>>     >     >
>>>     >     > case
>>>     >     >>> > > where
>>>     >     >>> > > > the author is not a committer (who could merge their
>>> own PR),
>>>     >     >
>>>     >     > should
>>>     >     >>> we
>>>     >     >>> > > > state that the reviewer will be responsible for the
>>> merge?
>>>     >     >>> > > >
>>>     >     >>> > > > -Kyle
>>>     >     >>> > > >
>>>     >     >>> > > > On Fri, Dec 16, 2016 at 6:39 PM, James Sirota <
>>> [email protected]>
>>>     >     >
>>>     >     >>> > > wrote:
>>>     >     >>> > > >
>>>     >     >>> > > >> Lets move this back to the discuss thread since
>>> it's still
>>>     >     >>> generating
>>>     >     >>> > > that
>>>     >     >>> > > >> many comments. Please post all your feedback and I
>>> will
>>>     >     >
>>>     >     > incorporate
>>>     >     >>> > it
>>>     >     >>> > > and
>>>     >     >>> > > >> put it back to a vote.
>>>     >     >>> > > >>
>>>     >     >>> > > >> Thanks,
>>>     >     >>> > > >> James
>>>     >     >>> > > >>
>>>     >     >>> > > >> 16.12.2016, 16:12, "Matt Foley" <[email protected]>:
>>>     >     >>> > > >> > +1
>>>     >     >>> > > >> >
>>>     >     >>> > > >> > In 2.2 (follow Sun guidelines), do you want to
>>> add the
>>>     >     >
>>>     >     > notation
>>>     >     >>> > > “except
>>>     >     >>> > > >> that indents are 2 spaces instead of 4”, as Hadoop
>>> does? Or does
>>>     >     >>> the
>>>     >     >>> > > Metron
>>>     >     >>> > > >> community like 4-space indents? I see both in the
>>> Metron code.
>>>     >     >>> > > >> >
>>>     >     >>> > > >> > My +1 holds in either case.
>>>     >     >>> > > >> > --Matt
>>>     >     >>> > > >> >
>>>     >     >>> > > >> > On 12/16/16, 9:34 AM, "James Sirota" <
>>> [email protected]>
>>>     >     >
>>>     >     > wrote:
>>>     >     >>> > > >> >
>>>     >     >>> > > >> > I incorporated the changes to the coding
>>> guidelines from our
>>>     >     >>> > discuss
>>>     >     >>> > > >> thread. I'd like to get them voted on to make them
>>> official.
>>>     >     >>> > > >> >
>>>     >     >>> > > >> > https://cwiki.apache.org/confl
>>> uence/pages/viewpage.
>>>     >     >>> > > >> action?pageId=61332235
>>>     >     >>> > > >> >
>>>     >     >>> > > >> > Please vote +1, -1, 0
>>>     >     >>> > > >> >
>>>     >     >>> > > >> > The vote will be open for 72 hours.
>>>     >     >>> > > >> >
>>>     >     >>> > > >> > -------------------
>>>     >     >>> > > >> > Thank you,
>>>     >     >>> > > >> >
>>>     >     >>> > > >> > James Sirota
>>>     >     >>> > > >> > PPMC- Apache Metron (Incubating)
>>>     >     >>> > > >> > jsirota AT apache DOT org
>>>     >     >>> > > >>
>>>     >     >>> > > >> -------------------
>>>     >     >>> > > >> Thank you,
>>>     >     >>> > > >>
>>>     >     >>> > > >> James Sirota
>>>     >     >>> > > >> PPMC- Apache Metron (Incubating)
>>>     >     >>> > > >> jsirota AT apache DOT org
>>>     >     >>> > >
>>>     >     >>> > > -------------------
>>>     >     >>> > > Thank you,
>>>     >     >>> > >
>>>     >     >>> > > James Sirota
>>>     >     >>> > > PPMC- Apache Metron (Incubating)
>>>     >     >>> > > jsirota AT apache DOT org
>>>     >     >>> > >
>>>     >     >>> > --
>>>     >     >>> >
>>>     >     >>> > Jon
>>>     >     >>> >
>>>     >     >>> > Sent from my mobile device
>>>     >     >>> >
>>>     >     >> --
>>>     >     >>
>>>     >     >> Jon
>>>     >     >>
>>>     >     >> Sent from my mobile device
>>>     >     >
>>>     >     > -------------------
>>>     >     > Thank you,
>>>     >     >
>>>     >     > James Sirota
>>>     >     > PPMC- Apache Metron (Incubating)
>>>     >     > jsirota AT apache DOT org
>>>     >
>>>     >     -------------------
>>>     >     Thank you,
>>>     >
>>>     >     James Sirota
>>>     >     PPMC- Apache Metron (Incubating)
>>>     >     jsirota AT apache DOT org
>>>
>>>     -------------------
>>>     Thank you,
>>>
>>>     James Sirota
>>>     PPMC- Apache Metron (Incubating)
>>>     jsirota AT apache DOT org
>>>
>>>
>>>
>>>
>>>
>>
>

Reply via email to