+1 on Casey’s first edit. However, wrt the second, can we please not require
vagrant? Any of our single-node test deployments, including vagrant, ansible,
mpack, or (soon :-) docker, should be acceptable.
Thanks,
--Matt (who can’t run vagrant workably on the systems available to me)
On 12/21/16, 8:52 AM, "Michael Miklavcic" <michael.miklav...@gmail.com> wrote:
Agreed on Casey's addition to 2.5. What do you think about saying the pla
should be stated on the PR, since that will be replicated to Jira
automatically?
On Wed, Dec 21, 2016 at 7:49 AM, Casey Stella <ceste...@gmail.com> wrote:
> 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 <ceste...@gmail.com> 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 <ceste...@gmail.com>
> wrote:
> >
> >> Yeah, I +1 the notion of thorough automated tests.
> >>
> >> On Tue, Dec 20, 2016 at 4:36 PM, Matt Foley <ma...@apache.org> 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" <jsir...@apache.org> wrote:
> >>>
> >>> Hi Matt, thats already in there. See last bullet point of 2.6
> >>>
> >>> 20.12.2016, 14:14, "Matt Foley" <ma...@apache.org>:
> >>> > 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" <jsir...@apache.org>
> 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 dev@metron.incubator.apache.org 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" <ottobackwa...@gmail.com>:
> >>> > > "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 (
> >>> jsir...@apache.org) 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 dev@metron.incubator.apache.org 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, "zeo...@gmail.com" <zeo...@gmail.com>:
> >>> > >> 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 <
> >>> > >> michael.miklav...@gmail.com> 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, "zeo...@gmail.com" <
> >>> zeo...@gmail.com> 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 <
> >>> jsir...@apache.org>
> >>> > >>> 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" <
> >>> kylerichards...@gmail.com>:
> >>> > >>> > > > 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 <
> >>> jsir...@apache.org>
> >>> > >
> >>> > >>> > > 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" <ma...@apache.org
> >:
> >>> > >>> > > >> > +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" <
> >>> jsir...@apache.org>
> >>> > >
> >>> > > 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
> >>>
> >>>
> >>>
> >>>
> >>>
> >>
> >
>
