Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#18792 closed enhancement (duplicate)

tweak build.xml for new tasks and java 7

Reported by: iwakeh Owned by: iwakeh
Priority: Medium Milestone: CollecTor 1.0.0
Component: Metrics/CollecTor Version:
Severity: Normal Keywords: ctip
Cc: Actual Points:
Parent ID: #18707 Points:
Reviewer: Sponsor:

Description

  • add clean, test, and jar ant-tasks
  • provide necessary properties and appropriate initialization for new tasks
  • switch to source/target java 1.7
  • provide version information for dependencies and use released descriptor
  • use lib folder for dependencies

Child Tickets

Attachments (2)

0001-implements-18792.patch (5.2 KB) - added by iwakeh 3 years ago.
patch
0001-implements-18792.patch​ (4.9 KB) - added by iwakeh 3 years ago.
new patch

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by iwakeh

Owner: changed from iwakehc to iwakeh
Status: newassigned

Changed 3 years ago by iwakeh

Attachment: 0001-implements-18792.patch added

patch

comment:2 Changed 3 years ago by iwakeh

Status: assignedneeds_review

Patch attached, please review.

Should there be empty lines between <target>...</target> blocks?

comment:3 Changed 3 years ago by karsten

Status: needs_reviewneeds_revision

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!

Changed 3 years ago by iwakeh

new patch

comment:4 in reply to:  3 Changed 3 years ago by iwakeh

Status: needs_revisionneeds_review

Thanks for the feedback!
I attached a new patch.

  • with a shorter comment
  • without the leading spaces you noticed, but some of these lines are actually new
  • changed the version to 0.9.0-dev

The jardoc jar is used by some IDEs for providing the javadoc. So, it will be useful.
I'll add javadoc jar to the release ticket (#18732) and your question about what to include in the executable jar to the corresponding ticket (#18719). Here I just wanted to 'modernize' the build tasks for the immediate further development in the ctip project.

(PS: I noticed that trac does not display syntax coloring for the new patch, but I cannot see why and the patch format seems ok.)

Last edited 3 years ago by iwakeh (previous) (diff)

comment:5 Changed 3 years ago by iwakeh

Status: needs_reviewneeds_revision

Good, that this is not reviewed yet!

Rather see the patch for the coverage ticket #18794.

Last edited 3 years ago by iwakeh (previous) (diff)

comment:6 Changed 3 years ago by iwakeh

Resolution: duplicate
Status: needs_revisionclosed

is now implemented by #18794.

comment:7 Changed 3 years ago by iwakeh

Milestone: CollecTor 1.0.0

Added to milestone for first release.

Note: See TracTickets for help on using tickets.