Re: [tor-bugs] #18792 [CollecTor]: tweak build.xml for new tasks and java 7

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#18792: tweak build.xml for new tasks and java 7
-------------------------+--------------------------------
 Reporter:  iwakeh       |          Owner:  iwakeh
     Type:  enhancement  |         Status:  needs_revision
 Priority:  Medium       |      Milestone:
Component:  CollecTor    |        Version:
 Severity:  Normal       |     Resolution:
 Keywords:  ctip         |  Actual Points:
Parent ID:  #18707       |         Points:
 Reviewer:               |        Sponsor:
-------------------------+--------------------------------
Changes (by karsten):

 * status:  needs_review => needs_revision


Comment:

 Thanks for writing this patch!  Please find some feedback below:

  - I'm trying (but not always succeeding) to keep Git commit messages at
 50 characters for the summary and 70 characters for the body.  That could
 be "Update build.xml to include new tasks." with the remainder of your
 commit message in the body.  I found that quite helpful when using `git
 log` or even `git blame`.  Does that make sense?  This could be yet one
 more guideline for metrics-team.git.

  - Should we start with version 0.9.0-dev while we're developing and
 change that to 1.0.0 just before the release?  Otherwise there's a small
 potential source of confusion with jar files produced while developing
 which look like they're the 1.0.0 release which is not the case.

  - Should we add another property `jardocsfile` and create such a .jar
 file with the docs?  (I don't know how useful such a .jar file would be,
 but I've seen other projects provide it, and I remember that we discussed
 adding it for metrics-lib a while back.)

  - Note to self: when we're merging this patch that makes `deps/metrics-
 lib/` obsolete, let's also delete/unregister the git submodule.

  - Should the produced .jar file contain the required .jar files from
 `lib/`, except for example `junit.jar`?  I don't remember the reasons
 for/against doing that in Onionoo and it might be that the answer is no
 for good reasons.  And should we remove the compiled ''test'' classes?
 See Onionoo's `build.xml` where we're doing that.

  - You added new whitespace to lines 38 to 45 which don't contain any
 other changes.  I would undo those changes, but I don't have any other
 changes to make right now, so it might be easiest for you to send a new
 patch with changes based on these comments.  Can you remove those leading
 spaces to keep the diff as small as possible?

  - Regarding your question where there should be empty lines between
 `<target>...</target>` blocks, I don't mind much as long as we're doing
 things consistently in a given code base.  Happy to keep it without empty
 lines as in your patch.

 Thanks again!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18792#comment:3>
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