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 > > >>> > > >>> > > >>> > > >>> > > >>> > > >> > > > > > > > > > >
