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 <[email protected]> wrote: > Should 2.5 specify the criteria for the requirement of integration level > tests? > > > On December 16, 2016 at 14:50:31, Otto Fowler ([email protected]) > 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 ( > [email protected]) 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 <[email protected]> > 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 ([email protected]) > 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 < > > [email protected]> > > > > 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 <[email protected]> > > 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 > > > > > > > > > >
