This question got lost in the discussion, but there is a small
improvement
that we can do:
Just to check, are we doing parallel builds?
We are on jenkins, not in travis, there is an ongoing PR to fix this.
What we can improve is to check if we can run some of the test suites
in
parallel to gain some extra time. For exemple most of the IOs and some
runners don't execute tests in parallel.
Ismael
(slightly related), is there a way to get change the timeout of travis
jobs). Because it is failing most of the time now because of this, and
it
is quite noisey to have so many false positives.
On Fri, Feb 10, 2017 at 8:00 PM, Robert Bradshaw <
rober...@google.com.invalid> wrote:
On Fri, Feb 10, 2017 at 8:45 AM, Dan Halperin
<dhalp...@google.com.invalid
wrote:
On Fri, Feb 10, 2017 at 7:42 AM, Kenneth Knowles
<k...@google.com.invalid
wrote:
On Feb 10, 2017 07:36, "Dan Halperin"
<dhalp...@google.com.invalid>
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.
Just to check, are we doing parallel builds?
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.
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").
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
