Works for me, thanks.

On 12/21/16, 11:21 AM, "Casey Stella" <ceste...@gmail.com> wrote:

    Sure, how about making it generic to "a deployed cluster"?
    
    On Wed, Dec 21, 2016 at 2:20 PM, Matt Foley <ma...@apache.org> wrote:
    
    > +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
    >     > >>>
    >     > >>>
    >     > >>>
    >     > >>>
    >     > >>>
    >     > >>
    >     > >
    >     >
    >
    >
    >
    >
    >
    



Reply via email to