Re: Better developer instructions for using Maven?

[Robert Bradshaw]

On Fri, Feb 10, 2017 at 11:54 AM, Davor Bonaci 
wrote:

> I think Dan's framework of thinking is right -- what is the probability of
> something finding a real issue, vs. the cost of running that all the time.
>
> Obviously, we cannot run *everything* all the time. There's an infinite
> number of things to run and infinite matrix of configurations. Many tests
> are not even possible to execute locally like testing scale, performance,
> compatibility across platforms, etc. So, we run locally *the least*,
> slightly more in presubmit, slightly more in post-submit, even more
> nightly, and as much as possible for each release. Of course, it is great
> to catch issues as soon as possible, but the cost is prohibitive in terms
> of time and productivity.
>
> This may sound a bit tangential so far... but, I think Aviem's (and others)
> point of view come from a social aspect. As a contributor, nobody wants to
> be "embarrassed" by proposing a pull request that is broken and has obvious
> problems. One might think of themselves as failing to do a good job in this
> particular case. So, instead, I might want to default to running more
> things locally to prevent the "embarrassment".
>

+1

It's also disheartening to think a PR is ready for folks to look at, only
to notice (later, when you've moved on to doing something else and a
Jenkins issue comes in) that you have, say, some trivial checkstyle
whitespace issue.

Also, in my experience, newcomers usually value direction and the ability
to algorithmically, privately verify their code quite a bit.

On the other hand, having to start up a spark cluster or create GCP
credentials is a higher burden that is often better offloaded to something
like Jenkins.

Looking more broadly, however, there's a huge difference in things that the
> default local execution *and* pre-commit don't exercise. It is really huge
> and many breakages are caught in post commit, nightlies, or, at the very
> end, cherrypicked into the release. So, I'd like to argue that nobody
> should ever be embarrassed by proposing a pull request. It is normal,
> expected, reasonable that Jenkins will catch issues -- that's why we have
> it. Instead, think in terms of "error budget" -- increase individual
> productivity by sacrificing occasional breakages. It is also completely
> normal that a PR causes a post-commit breakage at times, or that a release
> is delayed because of some PR -- this is all normal and fits into your
> "error budget".
>

In my opinion, post-commits should catch virtually nothing, or at
least nothing deterministic. Once we have a mergebot, there's no reason we
should't test virtually everything we do on those runs as a last step
before every merge instead/as well..

One easy improvement that I know Jason is looking at is to separate the
> precommit signal into multiple signals. So, instead of a one "red" signal,
> contributors can get five "greens" and one "red", which may help decrease
> this social impact.
>

That would be great.


> Hopefully, this is a convincing argument to use this framework in deciding
> this matter. On the lower-lever issue, I think rat and findbugs have a low
> probability of finding an issue in most cases.
>
> (Also, an explicit +1 to Kenn's point of view of getting people to the PR
> so we can work with them, as opposed to blocking them locally before they
> interact with us.)
>

We should also factor in how (naively) actionable the errors from the
various tools are. Linters are typically pretty explicit about what needs
to be changed, other tools sometimes less so.


> On Fri, Feb 10, 2017 at 11:50 AM, Kenneth Knowles 
> wrote:
>
> > Since the discussion has returned to the thread rather than Dan's PR, I
> > want to paraphrase the point I feel strongest about here:
> >
> > *For a new contributor, I want to minimize the distance between them
> > deciding to hack and becoming our friends.*
> >
> > So I don't want them to have to learn much, if anything, about our idioms
> > prior to opening a PR. That includes checkstyle, findbugs, javadoc, RAT,
> > (any others?). Once a PR is open, we can welcome them to the community,
> > help them understand any failure that happened via Jenkins, tell them the
> > command to run the more thorough test, or even issue little fix PRs
> against
> > their branch. If they get blocked by nitpicky or confusing failures in
> > their console or while hacking, they might (reasonably, IMO) decide to go
> > do something else.
> >
> > Folks other than newcomers can learn a repertoire of commands, like
> Robert
> > says. So we shouldn't consider them (aka "us") so much when deciding
> > whether "fast" or "slow" is the default, as long as we can explicitly
> > choose.
> >
> > Kenn
> >
> > On Fri, Feb 10, 2017 at 11:00 AM, Robert Bradshaw <
> > rober...@google.com.invalid> wrote:
> >
> > > > 1. Developer productivity -- Jenkins should run many more checks than
> > > > developers do. 

Re: Better developer instructions for using Maven?

[Davor Bonaci]

I think Dan's framework of thinking is right -- what is the probability of
something finding a real issue, vs. the cost of running that all the time.

Obviously, we cannot run *everything* all the time. There's an infinite
number of things to run and infinite matrix of configurations. Many tests
are not even possible to execute locally like testing scale, performance,
compatibility across platforms, etc. So, we run locally *the least*,
slightly more in presubmit, slightly more in post-submit, even more
nightly, and as much as possible for each release. Of course, it is great
to catch issues as soon as possible, but the cost is prohibitive in terms
of time and productivity.

This may sound a bit tangential so far... but, I think Aviem's (and others)
point of view come from a social aspect. As a contributor, nobody wants to
be "embarrassed" by proposing a pull request that is broken and has obvious
problems. One might think of themselves as failing to do a good job in this
particular case. So, instead, I might want to default to running more
things locally to prevent the "embarrassment".

Looking more broadly, however, there's a huge difference in things that the
default local execution *and* pre-commit don't exercise. It is really huge
and many breakages are caught in post commit, nightlies, or, at the very
end, cherrypicked into the release. So, I'd like to argue that nobody
should ever be embarrassed by proposing a pull request. It is normal,
expected, reasonable that Jenkins will catch issues -- that's why we have
it. Instead, think in terms of "error budget" -- increase individual
productivity by sacrificing occasional breakages. It is also completely
normal that a PR causes a post-commit breakage at times, or that a release
is delayed because of some PR -- this is all normal and fits into your
"error budget".

One easy improvement that I know Jason is looking at is to separate the
precommit signal into multiple signals. So, instead of a one "red" signal,
contributors can get five "greens" and one "red", which may help decrease
this social impact.

Hopefully, this is a convincing argument to use this framework in deciding
this matter. On the lower-lever issue, I think rat and findbugs have a low
probability of finding an issue in most cases.

(Also, an explicit +1 to Kenn's point of view of getting people to the PR
so we can work with them, as opposed to blocking them locally before they
interact with us.)

On Fri, Feb 10, 2017 at 11:50 AM, Kenneth Knowles 
wrote:

> Since the discussion has returned to the thread rather than Dan's PR, I
> want to paraphrase the point I feel strongest about here:
>
> *For a new contributor, I want to minimize the distance between them
> deciding to hack and becoming our friends.*
>
> So I don't want them to have to learn much, if anything, about our idioms
> prior to opening a PR. That includes checkstyle, findbugs, javadoc, RAT,
> (any others?). Once a PR is open, we can welcome them to the community,
> help them understand any failure that happened via Jenkins, tell them the
> command to run the more thorough test, or even issue little fix PRs against
> their branch. If they get blocked by nitpicky or confusing failures in
> their console or while hacking, they might (reasonably, IMO) decide to go
> do something else.
>
> Folks other than newcomers can learn a repertoire of commands, like Robert
> says. So we shouldn't consider them (aka "us") so much when deciding
> whether "fast" or "slow" is the default, as long as we can explicitly
> choose.
>
> Kenn
>
> On Fri, Feb 10, 2017 at 11:00 AM, Robert Bradshaw <
> rober...@google.com.invalid> wrote:
>
> > > 1. Developer productivity -- Jenkins should run many more checks than
> > > developers do. Especially time-, resource-, or setup- intensive tasks.
> > > 2. Automated enforcement -- Jenkins is better at running the right
> > commands
> > > than we are.
> > > 3. Lower the barrier to entry -- individual developers need not have a
> > > running Spark/Flink/Apex/Dataflow setup in order to contribute code.
> > > 4. Focus on the user -- someone checking out the code and using it for
> > the
> > > first time does not care whether the code style checks or has the right
> > > licenses -- that should have been enforced by the Beam team before
> > > committing.
> > >
> > > We should be *very* choosy about what we enforce on every developer
> every
> > > time they go to compile. I probably compile Beam 50x-100x a day.
> > Literally,
> > > the extra minutes you want to add here will cost me an hour daily.
> > >
> >
> > By the same token of having a different bar for the Jenkins presubmit vs.
> > what's run locally, I think it makes a lot of sense to run a different
> > command for iterative development than you run before creating a pull
> > request. E.g. during development I'll often run only one test rather than
> > the entire suite, but do run the entire suite occasionally (often before
> > commit, 

Re: Better developer instructions for using Maven?

[Kenneth Knowles]

Since the discussion has returned to the thread rather than Dan's PR, I
want to paraphrase the point I feel strongest about here:

*For a new contributor, I want to minimize the distance between them
deciding to hack and becoming our friends.*

So I don't want them to have to learn much, if anything, about our idioms
prior to opening a PR. That includes checkstyle, findbugs, javadoc, RAT,
(any others?). Once a PR is open, we can welcome them to the community,
help them understand any failure that happened via Jenkins, tell them the
command to run the more thorough test, or even issue little fix PRs against
their branch. If they get blocked by nitpicky or confusing failures in
their console or while hacking, they might (reasonably, IMO) decide to go
do something else.

Folks other than newcomers can learn a repertoire of commands, like Robert
says. So we shouldn't consider them (aka "us") so much when deciding
whether "fast" or "slow" is the default, as long as we can explicitly
choose.

Kenn

On Fri, Feb 10, 2017 at 11:00 AM, Robert Bradshaw <
rober...@google.com.invalid> wrote:

> > 1. Developer productivity -- Jenkins should run many more checks than
> > developers do. Especially time-, resource-, or setup- intensive tasks.
> > 2. Automated enforcement -- Jenkins is better at running the right
> commands
> > than we are.
> > 3. Lower the barrier to entry -- individual developers need not have a
> > running Spark/Flink/Apex/Dataflow setup in order to contribute code.
> > 4. Focus on the user -- someone checking out the code and using it for
> the
> > first time does not care whether the code style checks or has the right
> > licenses -- that should have been enforced by the Beam team before
> > committing.
> >
> > We should be *very* choosy about what we enforce on every developer every
> > time they go to compile. I probably compile Beam 50x-100x a day.
> Literally,
> > the extra minutes you want to add here will cost me an hour daily.
> >
>
> By the same token of having a different bar for the Jenkins presubmit vs.
> what's run locally, I think it makes a lot of sense to run a different
> command for iterative development than you run before creating a pull
> request. E.g. during development I'll often run only one test rather than
> the entire suite, but do run the entire suite occasionally (often before
> commit, especially before pushing).
>
> The contributors guild should give a suggested command to run before
> creating a PR, right in the docs of how to create a PR, which may be more
> expensive than what you run during development. IMHO, this should be fairly
> comprehensive (certainly tests and checkstyle, maybe javadoc and findbugs).
> This should be the "default" command that the one-time-contributor should
> know. For those compiling 50x or more a day, I think the burden of learning
> a second (or more) cheaper commands is not high, and we could even put such
> a thing in the docs (and hopefully a common maven convention like "mvn
> test").



Kenn


> I've listed the fraction of commits I think will break one of the following
> > if that property is not tested:
> >
> > * compiling (100%)
> > * tests (100%)
> > * checkstyle (90%)
> > * javadoc (30%)
> > * findbugs (5%)
> > * rat (1%)
> >
> > So you can see where I stand and why. I'm sorry that 1/20 PRs has Apache
> > RAT catch a licensing issue or Findbugs catch a threading issue -- you
> can
> > always get a larger set of the precommit checks using -Prelease, though
> of
> > course the integration tests and runnableonservice tests may catch more
> > issues still. But I want my developer minutes back for the 95%+ of the
> > rest.
> >
> > Dan
> >
>

ToString Method Name

[Jesse Anderson]

The ToString.of() violates the new transform rules and we need to choose a
new name.

Here is the method for reference:
  /**
   * Returns a {@code PTransform} which
transforms each
   * element of the input {@link PCollection} to a {@link String} using the
   * {@link Object#toString} method.
   */
  public static PTransform of() {
return new SimpleToString();
  }

Here are the possibilities we've had so far:

   - elements
   - default
   - simple
   - asString
   - simpleString
   - stringValue
   - toString
   - strings
   - make

I think default shouldn't be used as that's a keyword for Lambdas.

Here is the guide that I think of() is violating (@eugene is that correct?):
Name factory functions so that either the function name is a verb, or
referring to the transform reads like a verb: e.g. MongoDbIO.read(),
Flatten.iterables().

What are everyone's thoughts? I'm thinking going back to elements or make,
strings.

Thanks,

Jesse

Re: Better developer instructions for using Maven?

[Aljoscha Krettek]

I'm with Dan on this. The iteration time should be cut down as low as
possible and we have Jenkins to ensure that tests pass.

As a side note, there are IntelliJ plugins for Checkstyle and Findbugs and
my personal setup highlights Checkstyle violations as errors in the code so
I can immediately see them and fix them.

On Fri, 10 Feb 2017 at 17:45 Dan Halperin 
wrote:

> On Fri, Feb 10, 2017 at 7:42 AM, Kenneth Knowles 
> wrote:
>
> > On Feb 10, 2017 07:36, "Dan Halperin" 
> wrote:
> >
> > Before we added checkstyle it was under a minute. Now it's over five?
> > That's awful IMO
> >
> >
> > Checkstyle didn't cause all that, did it?
> >
>
> The "5 minutes" was going with Aviem's numbers after this change. But yes,
> Checkstyle alone substantially (>+50%) the time from what it was previously
> to adding it back to the default build.
>
> Noting that findbugs takes quite a lot more time. Javadoc and jar are the
> > other two slow ones.
> >
> > RAT is fast. But it has very poor error messages, so we wouldn't want a
> new
> > contributor trying to figure out what is going on without our help.
> >
>
> There is a larger philosophical issue here: is there a point of Jenkins
> precommit testing? Why not just make `mvn install` run everything that
> Jenkins does? For that matter, why don't committers just push directly to
> master? Wouldn't that make everyone's life easier?
>
> I'd argue that's not true.
>
> 1. Developer productivity -- Jenkins should run many more checks than
> developers do. Especially time-, resource-, or setup- intensive tasks.
> 2. Automated enforcement -- Jenkins is better at running the right commands
> than we are.
> 3. Lower the barrier to entry -- individual developers need not have a
> running Spark/Flink/Apex/Dataflow setup in order to contribute code.
> 4. Focus on the user -- someone checking out the code and using it for the
> first time does not care whether the code style checks or has the right
> licenses -- that should have been enforced by the Beam team before
> committing.
>
> We should be *very* choosy about what we enforce on every developer every
> time they go to compile. I probably compile Beam 50x-100x a day. Literally,
> the extra minutes you want to add here will cost me an hour daily.
>
> I've listed the fraction of commits I think will break one of the following
> if that property is not tested:
>
> * compiling (100%)
> * tests (100%)
> * checkstyle (90%)
> * javadoc (30%)
> * findbugs (5%)
> * rat (1%)
>
> So you can see where I stand and why. I'm sorry that 1/20 PRs has Apache
> RAT catch a licensing issue or Findbugs catch a threading issue -- you can
> always get a larger set of the precommit checks using -Prelease, though of
> course the integration tests and runnableonservice tests may catch more
> issues still. But I want my developer minutes back for the 95%+ of the
> rest.
>
> Dan
>
>
> >
> >
> > On Fri, Feb 10, 2017 at 07:14 Aviem Zur  wrote:
> >
> > > Opened JIRA ticket: https://issues.apache.org/jira/browse/BEAM-1457
> > >
> > > On Fri, Feb 10, 2017 at 4:54 PM Jean-Baptiste Onofré 
> > > wrote:
> > >
> > > > Yeah. Agree. Time extend is not huge and it's worth to add it in
> verify
> > > > phase.
> > > >
> > > > Regards
> > > > JB
> > > >
> > > > On Feb 10, 2017, 10:13, at 10:13, Aviem Zur 
> > wrote:
> > > > >This goes back to the original discussion in this thread - reduce
> the
> > > > >amount of things pull requesters should know and keep the maven
> > command
> > > > >in
> > > > >the PR checklist as: 'mvn clean verify'.
> > > > >
> > > > >So if rat and findbugs do not take that long to run I think they
> > should
> > > > >be
> > > > >run by 'mvn clean verify'
> > > > >
> > > > >I ran a quick test on my laptop to see how much time they add to the
> > > > >build
> > > > >(of the entire project):
> > > > >
> > > > >'mvn clean install -DskipTests' => Total time: 03:51 min
> > > > >'mvn clean install apache-rat:check findbugs:check -DskipTests'  =>
> > > > >Total
> > > > >time: 05:29 min (Added 01:38 min)
> > > > >'mvn clean install' => Total time: 09:37 min
> > > > >'mvn clean install apache-rat:check findbugs:check' => Total time:
> > > > >11:13
> > > > >min (Added 01:36 min)
> > > > >
> > > > >Are these times reasonable enough to add rat and findbugs to the
> > > > >default
> > > > >build?
> > > > >
> > > > >On Fri, Feb 10, 2017 at 1:55 PM Jean-Baptiste Onofré <
> j...@nanthrax.net
> > >
> > > > >wrote:
> > > > >
> > > > >> Hi
> > > > >>
> > > > >> We discussed about that at the beginning of the project. We agreed
> > to
> > > > >> execute rat and findbugs in a specific profile to reduce the build
> > > > >time for
> > > > >> dev.
> > > > >>
> > > > >> That's why I do mvn clean install -Prelease before submitting a PR
> > > > >and
> > > > >> just clean install when I'm developing.
> > > > >>
> > > > >> No problem to 

Re: Better developer instructions for using Maven?

[Kenneth Knowles]

On Feb 10, 2017 07:36, "Dan Halperin"  wrote:

Before we added checkstyle it was under a minute. Now it's over five?
That's awful IMO


Checkstyle didn't cause all that, did it?

Noting that findbugs takes quite a lot more time. Javadoc and jar are the
other two slow ones.

RAT is fast. But it has very poor error messages, so we wouldn't want a new
contributor trying to figure out what is going on without our help.


On Fri, Feb 10, 2017 at 07:14 Aviem Zur  wrote:

> Opened JIRA ticket: https://issues.apache.org/jira/browse/BEAM-1457
>
> On Fri, Feb 10, 2017 at 4:54 PM Jean-Baptiste Onofré 
> wrote:
>
> > Yeah. Agree. Time extend is not huge and it's worth to add it in verify
> > phase.
> >
> > Regards
> > JB
> >
> > On Feb 10, 2017, 10:13, at 10:13, Aviem Zur  wrote:
> > >This goes back to the original discussion in this thread - reduce the
> > >amount of things pull requesters should know and keep the maven command
> > >in
> > >the PR checklist as: 'mvn clean verify'.
> > >
> > >So if rat and findbugs do not take that long to run I think they should
> > >be
> > >run by 'mvn clean verify'
> > >
> > >I ran a quick test on my laptop to see how much time they add to the
> > >build
> > >(of the entire project):
> > >
> > >'mvn clean install -DskipTests' => Total time: 03:51 min
> > >'mvn clean install apache-rat:check findbugs:check -DskipTests'  =>
> > >Total
> > >time: 05:29 min (Added 01:38 min)
> > >'mvn clean install' => Total time: 09:37 min
> > >'mvn clean install apache-rat:check findbugs:check' => Total time:
> > >11:13
> > >min (Added 01:36 min)
> > >
> > >Are these times reasonable enough to add rat and findbugs to the
> > >default
> > >build?
> > >
> > >On Fri, Feb 10, 2017 at 1:55 PM Jean-Baptiste Onofré 
> > >wrote:
> > >
> > >> Hi
> > >>
> > >> We discussed about that at the beginning of the project. We agreed to
> > >> execute rat and findbugs in a specific profile to reduce the build
> > >time for
> > >> dev.
> > >>
> > >> That's why I do mvn clean install -Prelease before submitting a PR
> > >and
> > >> just clean install when I'm developing.
> > >>
> > >> No problem to change that.
> > >>
> > >> Regards
> > >> JB
> > >>
> > >> On Feb 10, 2017, 07:51, at 07:51, Aviem Zur 
> > >wrote:
> > >> >Can we consider adding rat-plugin and findbugs to the default verify
> > >> >phase?
> > >> >Currently they only run when the `release` profile is enabled.
> > >> >
> > >> >On Thu, Jan 26, 2017 at 11:42 AM Aljoscha Krettek
> > >
> > >> >wrote:
> > >> >
> > >> >> +1 to what Dan said
> > >> >>
> > >> >> On Wed, 25 Jan 2017 at 21:40 Kenneth Knowles
> > >
> > >> >> wrote:
> > >> >>
> > >> >> > +1
> > >> >> >
> > >> >> > On Jan 25, 2017 11:15, "Jean-Baptiste Onofré" 
> > >> >wrote:
> > >> >> >
> > >> >> > > +1
> > >> >> > >
> > >> >> > > It sounds good to me.
> > >> >> > >
> > >> >> > > Thanks Dan !
> > >> >> > >
> > >> >> > > Regards
> > >> >> > > JB⁣​
> > >> >> > >
> > >> >> > > On Jan 25, 2017, 19:39, at 19:39, Dan Halperin
> > >> >> > 
> > >> >> > > wrote:
> > >> >> > > >Here is my summary of the threads:
> > >> >> > > >
> > >> >> > > >Overwhelming agreement:
> > >> >> > > >
> > >> >> > > >- rename `release` to something more appropriate.
> > >> >> > > >- add `checkstyle` to the default build (it's basically a
> > >> >compile
> > >> >> > > >error)
> > >> >> > > >- add more information to contributor guide
> > >> >> > > >
> > >> >> > > >Reasonable agreement
> > >> >> > > >
> > >> >> > > >- don't update the github instructions to make passing `mvn
> > >> >verify
> > >> >> > > >-P > >> >> > > >checks>` mandatory. Maybe add a hint that this is a good
> > >proxy
> > >> >for
> > >> >> what
> > >> >> > > >Jenkins will run.
> > >> >> > > >
> > >> >> > > >Unresolved:
> > >> >> > > >
> > >> >> > > >- whether all checks should be in `mvn verify`
> > >> >> > > >- whether `mvn test` is useful for most workflows
> > >> >> > > >
> > >> >> > > >I'll propose to proceed with the overwhelmingly agreed-upon
> > >> >changes,
> > >> >> > > >and as
> > >> >> > > >we see increasingly many new contributors re-evaluate the
> > >> >remaining
> > >> >> > > >issues.
> > >> >> > > >
> > >> >> > > >Thanks,
> > >> >> > > >Dan
> > >> >> > > >
> > >> >> > > >On Tue, Jan 24, 2017 at 12:51 PM, Jean-Baptiste Onofré
> > >> >> > > >
> > >> >> > > >wrote:
> > >> >> > > >
> > >> >> > > >> +1 to at least update the contribution guide and improve
> > >the
> > >> >profile
> > >> >> > > >name.
> > >> >> > > >>
> > >> >> > > >> Regards
> > >> >> > > >> JB
> > >> >> > > >>
> > >> >> > > >>
> > >> >> > > >> On 01/24/2017 09:49 PM, Kenneth Knowles wrote:
> > >> >> > > >>
> > >> >> > > >>> My impression is that we don't have consensus on whether
> > >all
> > >> >checks
> > >> >> > > >or
> > >> >> > > >>> minimal checks 

Re: Better developer instructions for using Maven?

[Aviem Zur]

Opened JIRA ticket: https://issues.apache.org/jira/browse/BEAM-1457

On Fri, Feb 10, 2017 at 4:54 PM Jean-Baptiste Onofré 
wrote:

> Yeah. Agree. Time extend is not huge and it's worth to add it in verify
> phase.
>
> Regards
> JB
>
> On Feb 10, 2017, 10:13, at 10:13, Aviem Zur  wrote:
> >This goes back to the original discussion in this thread - reduce the
> >amount of things pull requesters should know and keep the maven command
> >in
> >the PR checklist as: 'mvn clean verify'.
> >
> >So if rat and findbugs do not take that long to run I think they should
> >be
> >run by 'mvn clean verify'
> >
> >I ran a quick test on my laptop to see how much time they add to the
> >build
> >(of the entire project):
> >
> >'mvn clean install -DskipTests' => Total time: 03:51 min
> >'mvn clean install apache-rat:check findbugs:check -DskipTests'  =>
> >Total
> >time: 05:29 min (Added 01:38 min)
> >'mvn clean install' => Total time: 09:37 min
> >'mvn clean install apache-rat:check findbugs:check' => Total time:
> >11:13
> >min (Added 01:36 min)
> >
> >Are these times reasonable enough to add rat and findbugs to the
> >default
> >build?
> >
> >On Fri, Feb 10, 2017 at 1:55 PM Jean-Baptiste Onofré 
> >wrote:
> >
> >> Hi
> >>
> >> We discussed about that at the beginning of the project. We agreed to
> >> execute rat and findbugs in a specific profile to reduce the build
> >time for
> >> dev.
> >>
> >> That's why I do mvn clean install -Prelease before submitting a PR
> >and
> >> just clean install when I'm developing.
> >>
> >> No problem to change that.
> >>
> >> Regards
> >> JB
> >>
> >> On Feb 10, 2017, 07:51, at 07:51, Aviem Zur 
> >wrote:
> >> >Can we consider adding rat-plugin and findbugs to the default verify
> >> >phase?
> >> >Currently they only run when the `release` profile is enabled.
> >> >
> >> >On Thu, Jan 26, 2017 at 11:42 AM Aljoscha Krettek
> >
> >> >wrote:
> >> >
> >> >> +1 to what Dan said
> >> >>
> >> >> On Wed, 25 Jan 2017 at 21:40 Kenneth Knowles
> >
> >> >> wrote:
> >> >>
> >> >> > +1
> >> >> >
> >> >> > On Jan 25, 2017 11:15, "Jean-Baptiste Onofré" 
> >> >wrote:
> >> >> >
> >> >> > > +1
> >> >> > >
> >> >> > > It sounds good to me.
> >> >> > >
> >> >> > > Thanks Dan !
> >> >> > >
> >> >> > > Regards
> >> >> > > JB⁣​
> >> >> > >
> >> >> > > On Jan 25, 2017, 19:39, at 19:39, Dan Halperin
> >> >> > 
> >> >> > > wrote:
> >> >> > > >Here is my summary of the threads:
> >> >> > > >
> >> >> > > >Overwhelming agreement:
> >> >> > > >
> >> >> > > >- rename `release` to something more appropriate.
> >> >> > > >- add `checkstyle` to the default build (it's basically a
> >> >compile
> >> >> > > >error)
> >> >> > > >- add more information to contributor guide
> >> >> > > >
> >> >> > > >Reasonable agreement
> >> >> > > >
> >> >> > > >- don't update the github instructions to make passing `mvn
> >> >verify
> >> >> > > >-P >> >> > > >checks>` mandatory. Maybe add a hint that this is a good
> >proxy
> >> >for
> >> >> what
> >> >> > > >Jenkins will run.
> >> >> > > >
> >> >> > > >Unresolved:
> >> >> > > >
> >> >> > > >- whether all checks should be in `mvn verify`
> >> >> > > >- whether `mvn test` is useful for most workflows
> >> >> > > >
> >> >> > > >I'll propose to proceed with the overwhelmingly agreed-upon
> >> >changes,
> >> >> > > >and as
> >> >> > > >we see increasingly many new contributors re-evaluate the
> >> >remaining
> >> >> > > >issues.
> >> >> > > >
> >> >> > > >Thanks,
> >> >> > > >Dan
> >> >> > > >
> >> >> > > >On Tue, Jan 24, 2017 at 12:51 PM, Jean-Baptiste Onofré
> >> >> > > >
> >> >> > > >wrote:
> >> >> > > >
> >> >> > > >> +1 to at least update the contribution guide and improve
> >the
> >> >profile
> >> >> > > >name.
> >> >> > > >>
> >> >> > > >> Regards
> >> >> > > >> JB
> >> >> > > >>
> >> >> > > >>
> >> >> > > >> On 01/24/2017 09:49 PM, Kenneth Knowles wrote:
> >> >> > > >>
> >> >> > > >>> My impression is that we don't have consensus on whether
> >all
> >> >checks
> >> >> > > >or
> >> >> > > >>> minimal checks should be the default, or whether we can
> >have
> >> >both
> >> >> > > >via `mvn
> >> >> > > >>> test` and `mvn verify`.
> >> >> > > >>>
> >> >> > > >>> But that doesn't prevent us from giving -P release a
> >better
> >> >name
> >> >> and
> >> >> > > >>> mentioning it in the dev guide and in some manner in our
> >PR
> >> >> > > >template.
> >> >> > > >>>
> >> >> > > >>> Right now we are living with the combination of the bad
> >> >aspects -
> >> >> > > >default
> >> >> > > >>> is not thorough but not actually fast and a thorough check
> >is
> >> >> > > >>> undocumented.
> >> >> > > >>>
> >> >> > > >>> On Tue, Jan 24, 2017 at 2:22 AM, Ismaël Mejía
> >> >
> >> >> > > >wrote:
> >> >> > > >>>
> >> >> > > >>> I just wanted to know if we have achieved some consensus
> >> >about
> >> 

Re: Better developer instructions for using Maven?

[Jean-Baptiste Onofré]

Yeah. Agree. Time extend is not huge and it's worth to add it in verify phase.

Regards
JB

On Feb 10, 2017, 10:13, at 10:13, Aviem Zur  wrote:
>This goes back to the original discussion in this thread - reduce the
>amount of things pull requesters should know and keep the maven command
>in
>the PR checklist as: 'mvn clean verify'.
>
>So if rat and findbugs do not take that long to run I think they should
>be
>run by 'mvn clean verify'
>
>I ran a quick test on my laptop to see how much time they add to the
>build
>(of the entire project):
>
>'mvn clean install -DskipTests' => Total time: 03:51 min
>'mvn clean install apache-rat:check findbugs:check -DskipTests'  =>
>Total
>time: 05:29 min (Added 01:38 min)
>'mvn clean install' => Total time: 09:37 min
>'mvn clean install apache-rat:check findbugs:check' => Total time:
>11:13
>min (Added 01:36 min)
>
>Are these times reasonable enough to add rat and findbugs to the
>default
>build?
>
>On Fri, Feb 10, 2017 at 1:55 PM Jean-Baptiste Onofré 
>wrote:
>
>> Hi
>>
>> We discussed about that at the beginning of the project. We agreed to
>> execute rat and findbugs in a specific profile to reduce the build
>time for
>> dev.
>>
>> That's why I do mvn clean install -Prelease before submitting a PR
>and
>> just clean install when I'm developing.
>>
>> No problem to change that.
>>
>> Regards
>> JB
>>
>> On Feb 10, 2017, 07:51, at 07:51, Aviem Zur 
>wrote:
>> >Can we consider adding rat-plugin and findbugs to the default verify
>> >phase?
>> >Currently they only run when the `release` profile is enabled.
>> >
>> >On Thu, Jan 26, 2017 at 11:42 AM Aljoscha Krettek
>
>> >wrote:
>> >
>> >> +1 to what Dan said
>> >>
>> >> On Wed, 25 Jan 2017 at 21:40 Kenneth Knowles
>
>> >> wrote:
>> >>
>> >> > +1
>> >> >
>> >> > On Jan 25, 2017 11:15, "Jean-Baptiste Onofré" 
>> >wrote:
>> >> >
>> >> > > +1
>> >> > >
>> >> > > It sounds good to me.
>> >> > >
>> >> > > Thanks Dan !
>> >> > >
>> >> > > Regards
>> >> > > JB⁣​
>> >> > >
>> >> > > On Jan 25, 2017, 19:39, at 19:39, Dan Halperin
>> >> > 
>> >> > > wrote:
>> >> > > >Here is my summary of the threads:
>> >> > > >
>> >> > > >Overwhelming agreement:
>> >> > > >
>> >> > > >- rename `release` to something more appropriate.
>> >> > > >- add `checkstyle` to the default build (it's basically a
>> >compile
>> >> > > >error)
>> >> > > >- add more information to contributor guide
>> >> > > >
>> >> > > >Reasonable agreement
>> >> > > >
>> >> > > >- don't update the github instructions to make passing `mvn
>> >verify
>> >> > > >-P> >> > > >checks>` mandatory. Maybe add a hint that this is a good
>proxy
>> >for
>> >> what
>> >> > > >Jenkins will run.
>> >> > > >
>> >> > > >Unresolved:
>> >> > > >
>> >> > > >- whether all checks should be in `mvn verify`
>> >> > > >- whether `mvn test` is useful for most workflows
>> >> > > >
>> >> > > >I'll propose to proceed with the overwhelmingly agreed-upon
>> >changes,
>> >> > > >and as
>> >> > > >we see increasingly many new contributors re-evaluate the
>> >remaining
>> >> > > >issues.
>> >> > > >
>> >> > > >Thanks,
>> >> > > >Dan
>> >> > > >
>> >> > > >On Tue, Jan 24, 2017 at 12:51 PM, Jean-Baptiste Onofré
>> >> > > >
>> >> > > >wrote:
>> >> > > >
>> >> > > >> +1 to at least update the contribution guide and improve
>the
>> >profile
>> >> > > >name.
>> >> > > >>
>> >> > > >> Regards
>> >> > > >> JB
>> >> > > >>
>> >> > > >>
>> >> > > >> On 01/24/2017 09:49 PM, Kenneth Knowles wrote:
>> >> > > >>
>> >> > > >>> My impression is that we don't have consensus on whether
>all
>> >checks
>> >> > > >or
>> >> > > >>> minimal checks should be the default, or whether we can
>have
>> >both
>> >> > > >via `mvn
>> >> > > >>> test` and `mvn verify`.
>> >> > > >>>
>> >> > > >>> But that doesn't prevent us from giving -P release a
>better
>> >name
>> >> and
>> >> > > >>> mentioning it in the dev guide and in some manner in our
>PR
>> >> > > >template.
>> >> > > >>>
>> >> > > >>> Right now we are living with the combination of the bad
>> >aspects -
>> >> > > >default
>> >> > > >>> is not thorough but not actually fast and a thorough check
>is
>> >> > > >>> undocumented.
>> >> > > >>>
>> >> > > >>> On Tue, Jan 24, 2017 at 2:22 AM, Ismaël Mejía
>> >
>> >> > > >wrote:
>> >> > > >>>
>> >> > > >>> I just wanted to know if we have achieved some consensus
>> >about
>> >> this,
>> >> > > >I
>> >> > >  just
>> >> > >  saw this PR that reminded me about this discussion.
>> >> > > 
>> >> > >  ​https://github.com/apache/beam/pull/1829​
>> >> > > 
>> >> > >  It is important that we mention the existing profiles
>(and
>> >the
>> >> > > >intended
>> >> > >  checks) in the contribution guide (e.g. -Prelease (or
>> >-Pall-checks
>> >> > >  triggers
>> >> > >  these validations).
>> >> > > 
>> >> > >  I can add