Re: [DISCUSS] Coding Guidelines

Does anyone else has any comments or objections to the document? Or can we put this up for a vote? 21.12.2016, 13:24, "Kyle Richardson" : > +1 Thanks for the updates to the documentation and testing sections. > > -Kyle > > On Wed, Dec 21, 2016 at 3:17 PM, Otto Fowler

Re: [DISCUSS] Coding Guidelines

+1 On December 21, 2016 at 14:56:06, Michael Miklavcic ( michael.miklav...@gmail.com) wrote: Works for me also. On Wed, Dec 21, 2016 at 12:38 PM, Matt Foley wrote: > Works for me, thanks. > > On 12/21/16, 11:21 AM, "Casey Stella" wrote: > > Sure, how

Re: [DISCUSS] Coding Guidelines

Works for me also. On Wed, Dec 21, 2016 at 12:38 PM, Matt Foley wrote: > Works for me, thanks. > > On 12/21/16, 11:21 AM, "Casey Stella" wrote: > > Sure, how about making it generic to "a deployed cluster"? > > On Wed, Dec 21, 2016 at 2:20 PM, Matt

Re: [DISCUSS] Coding Guidelines

Works for me, thanks. On 12/21/16, 11:21 AM, "Casey Stella" wrote: Sure, how about making it generic to "a deployed cluster"? On Wed, Dec 21, 2016 at 2:20 PM, Matt Foley wrote: > +1 on Casey’s first edit. However, wrt the second, can

Re: [DISCUSS] Coding Guidelines

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

Re: [DISCUSS] Coding Guidelines

+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

Re: [DISCUSS] Coding Guidelines

Agreed on Casey's addition to 2.5. What do you think about saying the plan 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 wrote: > Oh, one more, I propose the following addition to 2.5: > > > >

Re: [DISCUSS] Coding Guidelines

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

Re: [DISCUSS] Coding Guidelines

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

Re: [DISCUSS] Coding Guidelines

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 …

Re: [DISCUSS] Coding Guidelines

Hi Matt, thats already in there. See last bullet point of 2.6 20.12.2016, 14:14, "Matt Foley" : > 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

Re: [DISCUSS] Coding Guidelines

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

Re: [DISCUSS] Coding Guidelines

Good feedback. Here is the next iteration that accounts for your suggestions: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=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

Re: [DISCUSS] Coding Guidelines

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

Re: [DISCUSS] Coding Guidelines

Once the pull request is merged, manually close the corresponding JIRA Again, the submitter may not have rights to assign themselves the jira to close, even if they have rights to create.  We need to be clearer on this part of the process. Does the person doing the merge assign the jira to

Re: [DISCUSS] Coding Guidelines

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

Re: [DISCUSS] Coding Guidelines

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" wrote: > Regarding documentation - while I'm not a huge fan of that approach (I > would prefer to see

Re: [DISCUSS] Coding Guidelines

+1 to that. On Tue, Dec 20, 2016 at 11:23 Michael Miklavcic wrote: > Just want to make sure the change to remove "100% code coverage" makes it > > in. I prefer something along the lines of the language Otto and I were > > discussing: > > > > "All merged patches will

Re: [DISCUSS] Coding Guidelines

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

Re: [DISCUSS] Coding Guidelines

Given that we have been seeing problems that appear only in travis-ci, it would seem prudent that the project encourage if not require pull request submitters to link to travis-ci and test their builds. On November 29, 2016 at 16:20:59, James Sirota (jsir...@apache.org) wrote: We have a really

Re: [DISCUSS] Coding Guidelines

Apache Hadoop does in fact use checkstyle and the maven checkstyle plugin, see https://wiki.apache.org/hadoop/HowToContribute#Contributing_your_work and https://github.com/apache/hadoop/blob/trunk/pom.xml . The checkstyle configuration files are in

Re: [DISCUSS] Coding Guidelines

>From what I can tell, it's not up to date. For instance the doc mentions that a committer may never merge their own PR, but I know that has become somewhat standard practice recently, as long as it has been properly reviewed. In addition, the recent transient build issues have been forcing us

Re: [DISCUSS] Coding Guidelines

Are the committing guidelines up to date? When I stated committing someone mentioned that they were not. On November 29, 2016 at 16:20:59, James Sirota (jsir...@apache.org) wrote: We have a really old (and incomplete) coding guidelines document that I'd like to clean up prior to our

Re: [DISCUSS] Coding Guidelines

+1 to checkstyle. Maybe even tackle that effort project-by-project instead of one giant PR. But however it gets done, its got to get done. On Tue, Nov 29, 2016 at 6:23 PM, Michael Miklavcic < michael.miklav...@gmail.com> wrote: > For 3, I think we should implement checkstyle, perform a

Re: [DISCUSS] Coding Guidelines

1. I think we need to emphasize good unit testing more. I guess METRON-37 integrated Cobertura with our build process, but how often does it run and where is JUnit code coverage reported? 2. Regarding the missing Code Style Guidelines, I’ve always thought the Hadoop Style guide was good: