[tor-bugs] #20596 [Metrics]: streamline build.xml and metrics_checkstyle.xml throughout all java projects

Tue Jan 3 17:18:45 UTC 2017

#20596: streamline build.xml and metrics_checkstyle.xml throughout all java
projects
-------------------------+------------------------------
 Reporter:  iwakeh       |          Owner:  iwakeh
     Type:  enhancement  |         Status:  needs_review
 Priority:  Medium       |      Milestone:
Component:  Metrics      |        Version:
 Severity:  Normal       |     Resolution:
 Keywords:               |  Actual Points:
Parent ID:               |         Points:
 Reviewer:               |        Sponsor:
-------------------------+------------------------------
Changes (by iwakeh):

 * status:  needs_revision => needs_review

Comment:

 Replying to [comment:29 karsten]:
 > Whee, quite some code to review here.  Sure, a lot of code has just
 moved around, but that only makes it trickier to spot the little
 differences. ...

 Yes, that is how it feels to code this stuff ;-)
 I updated all branches and will attach the patches for metrics-base here.
 More in-line.

 > ...  But hopefully this initial feedback is useful anyway:

 Very useful!  Such a tasks just needs lots of testing/reviewing.

 >
 > https://gitweb.torproject.org/metrics-
 base.git/commit/?id=7a01d68f813cb5ae2904e73dbc81999ce0622eca
 >
 >  - The two properties `"libs"` and `"source-and-target-java-version"`
 are listed twice.

 Removed.

 >
 >  - Speaking of `"source-and-target-java-version"`, we should probably
 keep that property project-specific.  Otherwise we'll have to raise the
 Java version for all projects at once, including metrics-lib and projects
 depending on metrics-lib.

 If a project deviates from the base java version, only the property needs
 to be added to build.xml and takes precedence.  (Try setting it to java
 1.4 and see how the compile complains about generics.)

 I added a comment to the build.template.xml

 >
 >  - The Checkstyle configuration file `java/metrics_checks.xml` does not
 contain modules `SuppressWarningsFilter` and `SuppressWarningsHolder` that
 we added in metrics-lib's version nor module `UnusedImports` in Onionoo's
 version.  It also still defines a maximum line length of 100 rather than
 80.  Let's combine the latest settings from all three code bases here.

 Changed the line length check; all others were already part of the initial
 check-in.

 >
 >  - This is certainly a minor issue, but the usage instructions in the
 `usage` target should all use the same capitalization and sentence
 structure, like "'name' does XY", not "'name' Does XY" or "'name' XY".
 >
 >  - It's already time to update the copyright year in the `docs` target!
 Happy 2017!

 Good that this only needs a change in one place now :-)
 The year is now computed from the current time UTC.

 >
 > https://trac.torproject.org/projects/tor/attachment/ticket/20596/0001
 -Implements-part-of-task-20596.patch
 >
 >  - Typo in `java/build.xml.template`: "jarpatterprop" is missing an "n".

 Fixed.

 >
 >  - By the way, I already pushed this commit to facilitate testing.  We
 can fix things in subsequent commits.

 Very useful.

 >
 >
 https://gitweb.torproject.org/user/iwakeh/onionoo.git/commit/?h=task-20596-submod&id=0442032f24c8764d6c055ed3b9f4e047fbaa886e
 >
 >  - Why did you change the `"jetty.version"` property to `""` rather than
 `"-8.1.16.v20140903"`?

 Fixed.

 >
 >  - The new generic `tar` target does not include the `DESIGN` file
 anymore.  I'd argue that we should remove that file anyway, but if you
 want to keep it, we'll have to include it in the release tarball, too.

 Removed.  Designs should be elsewhere, for example Tech-Reports.

 >
 >  - The script file `src/main/resources/bootstrap-development.sh` should
 be checked in with the executable flag.

 Done.

 >
 >  - This commit or a subsequent commit should remove
 `src/test/resources/metrics_checks.xml` which is now obsolete.

 Done.

 >
 > https://gitweb.torproject.org/user/iwakeh/metrics-
 lib.git/commit/?h=task-20596-submod&id=60d20b85c4dac3d9d0905a185b7bc928736f7fcf
 >
 >  - Which tests did not pass prior to this commit?  They run through just
 fine here.

 Hmm, now these tests pass here, too.  I'll go hunting for the
 jdk/environment setting responsible, but this shouldn't affect this
 ticket.  Please, just cherry pick.

 >
 >  - It shouldn't be necessary to initialize attributes with `null` or
 `0`, which is the default anyway.  All new no-args constructors should be
 empty (except for a comment saying that they're empty on purpose).

 The compiler complains about uninitialized fields (those are final in the
 respective classes).  Otherwise, same as the previous.

 >
 > https://gitweb.torproject.org/user/iwakeh/metrics-
 lib.git/commit/?h=task-20596-submod&id=946c403c357661a2290b424ecb1a1c70d70c1334
 >
 >  - The script file `src/main/resources/bootstrap-development.sh` should
 be checked in with the executable flag.

 Done.

 >
 >  - This commit or a subsequent commit should remove
 `src/test/resources/metrics_checks.xml` which is now obsolete.

 Done.

 >
 > https://gitweb.torproject.org/user/iwakeh/metrics-
 lib.git/commit/?h=task-20596-submod&id=7903d6a55504801dd03c75867ce79ff8f3799f11
 >
 >  - Looks good!

 Yeah!

 >
 >
 https://gitweb.torproject.org/user/iwakeh/collector.git/commit/?h=task-20596-submod&id=277be1b5802f8a7408fad27748ca00dab802d54f
 >
 >  - The script file `src/main/resources/bootstrap-development.sh` should
 be checked in with the executable flag.

 Done.

 >
 >  - This commit or a subsequent commit should remove
 `src/test/resources/junittest.policy` and
 `src/test/resources/metrics_checks.xml` which are now obsolete.

 Done.

 >
 > I guess I'll wait for the revision before trying out these changes
 altogether.  My plan was to create release tarballs and compare them with
 the latest releases.

 Be aware, that Onionoo '''does not''' include the protocol version in the
 manifest anymore, because the protocol version is part of the release
 version now.

 In addition, also simulate development tasks.

 >
 > Thanks again for making build processes better!

 Thanks for plowing to all these changes!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/20596#comment:30>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online