You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by James Sirota <js...@apache.org> on 2016/12/16 17:34:30 UTC

[VOTE] Coding Guidelines

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

[CANCEL] [VOTE] Coding Guidelines

Posted by James Sirota <js...@apache.org>.

16.12.2016, 13:11, "P. Taylor Goetz" <pt...@gmail.com>:
> Just a procedural note:
>
> When voting on a mutable document (like a wiki page), you should attach the document as text to the vote email since it\u2019s possible that the wiki content might change during the vote. It will also document exactly what was voted on for historical purposes.
>
> -Taylor
>
>> �On Dec 16, 2016, at 12:34 PM, James Sirota <js...@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/confluence/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

Re: [VOTE] Coding Guidelines

Posted by James Sirota <js...@apache.org>.
Will do.  I'll cancel the vote and attach text 

16.12.2016, 13:11, "P. Taylor Goetz" <pt...@gmail.com>:
> Just a procedural note:
>
> When voting on a mutable document (like a wiki page), you should attach the document as text to the vote email since it\u2019s possible that the wiki content might change during the vote. It will also document exactly what was voted on for historical purposes.
>
> -Taylor
>
>> �On Dec 16, 2016, at 12:34 PM, James Sirota <js...@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/confluence/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

Re: [VOTE] Coding Guidelines

Posted by "P. Taylor Goetz" <pt...@gmail.com>.
Just a procedural note:

When voting on a mutable document (like a wiki page), you should attach the document as text to the vote email since it’s possible that the wiki content might change during the vote. It will also document exactly what was voted on for historical purposes.

-Taylor

> On Dec 16, 2016, at 12:34 PM, James Sirota <js...@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/confluence/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


Re: [VOTE] Coding Guidelines

Posted by Casey Stella <ce...@gmail.com>.
I think this vote might be premature given the discussion on the vote
thread.  Can we move this back to a discuss and hash this out?  As it
stands, I'm -1 on "All merged patches must have 100% test coverage."

On Fri, Dec 16, 2016 at 2:51 PM, Otto Fowler <ot...@gmail.com>
wrote:

> Should 2.5 specify the criteria for the requirement of integration level
> tests?
>
>
> On December 16, 2016 at 14:50:31, Otto Fowler (ottobackwards@gmail.com)
> wrote:
>
> We can say that as part of the submittal ( until we think we need and
> actual template ), the pr should contain the methodology used to test and
> verify the pr.
>
>
> On December 16, 2016 at 14:42:48, Michael Miklavcic (
> michael.miklavcic@gmail.com) wrote:
>
> I like that Otto. I'd like to tweak it just a bit to say automated tests. I
> also tend to manually/smoke test things, but I'm on the fence if we want to
> specify that as well. Just for example, with the new HyperLogLogPlus
> Stellar functions I added, there are unit and integration tests, but I also
> spun up the REPL to make sure everything works as expected.
>
> “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 )"
>
> On Fri, Dec 16, 2016 at 12:36 PM, Otto Fowler <ot...@gmail.com>
> wrote:
>
> > Also, this doesn’t specify the acceptable way to measure that.
> >
> > The question is how to properly phrase it.
> >
> > “All merged patches will be reviewed with the expectation that tests
> exist
> > and are consistent with project testing methodology and practices, and
> > cover the appropriate cases ( see reviewers guide )"
> >
> >
> > On December 16, 2016 at 13:58:14, Casey Stella (cestella@gmail.com)
> wrote:
> >
> > Yeah I don't like that requirement at all. Sensible unit and integration
> > test representation is a decent goal, but I don't like a code coverage
> req.
> > On Fri, Dec 16, 2016 at 13:48 Michael Miklavcic <
> > michael.miklavcic@gmail.com>
> >
> > wrote:
> >
> > > Can someone clarify this point in merge requirements?
> > >
> > > "All merged patches must have 100% test coverage."
> > >
> > >
> > > On Fri, Dec 16, 2016 at 10:34 AM, James Sirota <js...@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/confluence/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
> > > >
> > >
> >
>

Re: [VOTE] Coding Guidelines

Posted by Otto Fowler <ot...@gmail.com>.
Should 2.5 specify the criteria for the requirement of integration level
tests?


On December 16, 2016 at 14:50:31, Otto Fowler (ottobackwards@gmail.com)
wrote:

We can say that as part of the submittal ( until we think we need and
actual template ), the pr should contain the methodology used to test and
verify the pr.


On December 16, 2016 at 14:42:48, Michael Miklavcic (
michael.miklavcic@gmail.com) wrote:

I like that Otto. I'd like to tweak it just a bit to say automated tests. I
also tend to manually/smoke test things, but I'm on the fence if we want to
specify that as well. Just for example, with the new HyperLogLogPlus
Stellar functions I added, there are unit and integration tests, but I also
spun up the REPL to make sure everything works as expected.

“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 )"

On Fri, Dec 16, 2016 at 12:36 PM, Otto Fowler <ot...@gmail.com>
wrote:

> Also, this doesn’t specify the acceptable way to measure that.
>
> The question is how to properly phrase it.
>
> “All merged patches will be reviewed with the expectation that tests exist
> and are consistent with project testing methodology and practices, and
> cover the appropriate cases ( see reviewers guide )"
>
>
> On December 16, 2016 at 13:58:14, Casey Stella (cestella@gmail.com) wrote:
>
> Yeah I don't like that requirement at all. Sensible unit and integration
> test representation is a decent goal, but I don't like a code coverage
req.
> On Fri, Dec 16, 2016 at 13:48 Michael Miklavcic <
> michael.miklavcic@gmail.com>
>
> wrote:
>
> > Can someone clarify this point in merge requirements?
> >
> > "All merged patches must have 100% test coverage."
> >
> >
> > On Fri, Dec 16, 2016 at 10:34 AM, James Sirota <js...@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/confluence/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
> > >
> >
>

Re: [VOTE] Coding Guidelines

Posted by Otto Fowler <ot...@gmail.com>.
We can say that as part of the submittal ( until we think we need and
actual template ), the pr should contain the methodology used to test and
verify the pr.


On December 16, 2016 at 14:42:48, Michael Miklavcic (
michael.miklavcic@gmail.com) wrote:

I like that Otto. I'd like to tweak it just a bit to say automated tests. I
also tend to manually/smoke test things, but I'm on the fence if we want to
specify that as well. Just for example, with the new HyperLogLogPlus
Stellar functions I added, there are unit and integration tests, but I also
spun up the REPL to make sure everything works as expected.

“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 )"

On Fri, Dec 16, 2016 at 12:36 PM, Otto Fowler <ot...@gmail.com>
wrote:

> Also, this doesn’t specify the acceptable way to measure that.
>
> The question is how to properly phrase it.
>
> “All merged patches will be reviewed with the expectation that tests
exist
> and are consistent with project testing methodology and practices, and
> cover the appropriate cases ( see reviewers guide )"
>
>
> On December 16, 2016 at 13:58:14, Casey Stella (cestella@gmail.com)
wrote:
>
> Yeah I don't like that requirement at all. Sensible unit and integration
> test representation is a decent goal, but I don't like a code coverage
req.
> On Fri, Dec 16, 2016 at 13:48 Michael Miklavcic <
> michael.miklavcic@gmail.com>
>
> wrote:
>
> > Can someone clarify this point in merge requirements?
> >
> > "All merged patches must have 100% test coverage."
> >
> >
> > On Fri, Dec 16, 2016 at 10:34 AM, James Sirota <js...@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/confluence/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
> > >
> >
>

Re: [VOTE] Coding Guidelines

Posted by Otto Fowler <ot...@gmail.com>.
Also the line for reviewers:

   - Are there sufficient tests?


“Are there sufficient tests of the correct type ( unit, integration )?  Are
the tests consistent with the project’s testing methodology?"

On December 16, 2016 at 14:42:48, Michael Miklavcic (
michael.miklavcic@gmail.com) wrote:

I like that Otto. I'd like to tweak it just a bit to say automated tests. I
also tend to manually/smoke test things, but I'm on the fence if we want to
specify that as well. Just for example, with the new HyperLogLogPlus
Stellar functions I added, there are unit and integration tests, but I also
spun up the REPL to make sure everything works as expected.

“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 )"

On Fri, Dec 16, 2016 at 12:36 PM, Otto Fowler <ot...@gmail.com>
wrote:

> Also, this doesn’t specify the acceptable way to measure that.
>
> The question is how to properly phrase it.
>
> “All merged patches will be reviewed with the expectation that tests
exist
> and are consistent with project testing methodology and practices, and
> cover the appropriate cases ( see reviewers guide )"
>
>
> On December 16, 2016 at 13:58:14, Casey Stella (cestella@gmail.com)
wrote:
>
> Yeah I don't like that requirement at all. Sensible unit and integration
> test representation is a decent goal, but I don't like a code coverage
req.
> On Fri, Dec 16, 2016 at 13:48 Michael Miklavcic <
> michael.miklavcic@gmail.com>
>
> wrote:
>
> > Can someone clarify this point in merge requirements?
> >
> > "All merged patches must have 100% test coverage."
> >
> >
> > On Fri, Dec 16, 2016 at 10:34 AM, James Sirota <js...@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/confluence/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
> > >
> >
>

Re: [VOTE] Coding Guidelines

Posted by Michael Miklavcic <mi...@gmail.com>.
I like that Otto. I'd like to tweak it just a bit to say automated tests. I
also tend to manually/smoke test things, but I'm on the fence if we want to
specify that as well. Just for example, with the new HyperLogLogPlus
Stellar functions I added, there are unit and integration tests, but I also
spun up the REPL to make sure everything works as expected.

“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 )"

On Fri, Dec 16, 2016 at 12:36 PM, Otto Fowler <ot...@gmail.com>
wrote:

> Also, this doesn’t specify the acceptable way to measure that.
>
> The question is how to properly phrase it.
>
> “All merged patches will be reviewed with the expectation that tests exist
> and are consistent with project testing methodology and practices, and
> cover the appropriate cases ( see reviewers guide )"
>
>
> On December 16, 2016 at 13:58:14, Casey Stella (cestella@gmail.com) wrote:
>
> Yeah I don't like that requirement at all. Sensible unit and integration
> test representation is a decent goal, but I don't like a code coverage req.
> On Fri, Dec 16, 2016 at 13:48 Michael Miklavcic <
> michael.miklavcic@gmail.com>
>
> wrote:
>
> > Can someone clarify this point in merge requirements?
> >
> > "All merged patches must have 100% test coverage."
> >
> >
> > On Fri, Dec 16, 2016 at 10:34 AM, James Sirota <js...@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/confluence/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
> > >
> >
>

Re: [VOTE] Coding Guidelines

Posted by Otto Fowler <ot...@gmail.com>.
Also, this doesn’t specify the acceptable way to measure that.

The question is how to properly phrase it.

“All merged patches will be reviewed with the expectation that tests exist
and are consistent with project testing methodology and practices, and
cover the appropriate cases ( see reviewers guide )"


On December 16, 2016 at 13:58:14, Casey Stella (cestella@gmail.com) wrote:

Yeah I don't like that requirement at all. Sensible unit and integration
test representation is a decent goal, but I don't like a code coverage req.
On Fri, Dec 16, 2016 at 13:48 Michael Miklavcic <mi...@gmail.com>

wrote:

> Can someone clarify this point in merge requirements?
>
> "All merged patches must have 100% test coverage."
>
>
> On Fri, Dec 16, 2016 at 10:34 AM, James Sirota <js...@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/confluence/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
> >
>

Re: [VOTE] Coding Guidelines

Posted by Casey Stella <ce...@gmail.com>.
Yeah I don't like that requirement at all. Sensible unit and integration
test representation is a decent goal, but I don't like a code coverage req.
On Fri, Dec 16, 2016 at 13:48 Michael Miklavcic <mi...@gmail.com>
wrote:

> Can someone clarify this point in merge requirements?
>
> "All merged patches must have 100% test coverage."
>
>
> On Fri, Dec 16, 2016 at 10:34 AM, James Sirota <js...@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/confluence/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
> >
>

Re: [VOTE] Coding Guidelines

Posted by Michael Miklavcic <mi...@gmail.com>.
Can someone clarify this point in merge requirements?

"All merged patches must have 100% test coverage."


On Fri, Dec 16, 2016 at 10:34 AM, James Sirota <js...@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/confluence/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
>