[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #20596 [Metrics]: streamline build.xml and metrics_checkstyle.xml throughout all java projects
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs