Skip to content

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Jan 14, 2015

No description provided.

@ppalaga
Copy link
Contributor Author

ppalaga commented Jan 19, 2015

This PR introduces a license header template for com.mycila:license-maven-plugin. Please refer to this branch to see how the template is supposed to be consumed by projects: https://github.com/ppalaga/rhq-metrics/tree/150116-license

There are two use cases:
(1) Fix or add the license headers in all project's files through issuing mvn license:format
(2) Check the presence of the headers and their correctness through mvn license:check. Note that license:check is bound to the validation phase https://github.com/ppalaga/rhq-metrics/blob/150116-license/pom.xml#L491

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment

AFAIC, I don't see the value in extracting all versions to properties (I do see value when the version number is used twice or more)

Is this a rule for POM writing we're supposed to obey or only your own practice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that's how it is done - not only usually but also in the Hawkular family. It is practical to do things in uniform ways so that one can find them quickly in a well defined location. Moreover, doing it the same way in the whole family of hawkular projects will make it easier to keep the same versions on multiple place. As far as I can see now, it will be inevitable to copy e.g. com.puppycrawl.tools.checkstyle version between here and the hawkular ur-parent.
You are right that there is no re-use in this particular place but for the named reasons, I'd keep it like this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that's how it is done - not only usually but also in the Hawkular family

If it's been agreed then ok.

@tsegismont
Copy link
Contributor

I did not really take the time to look at the details of the JGit code, I trust you for the how and your JUnit tests for the correctness.

I have one question on the license:check goal being attached to the build by default: how does that impact build time?

Also, it means that it's no longer possible to download the sources without Git and build the project: a Git repository becomes mandatory.

@ppalaga
Copy link
Contributor Author

ppalaga commented Jan 20, 2015

license:check goal being attached to the build by default: how does that impact build time?

I have no exact numbers. The subjective feeling is that I have not noticed any substantial delay. Should I try to get some exact numbers through running with and without license checks?

no longer possible to download the sources without Git and build the project: a Git repository becomes mandatory.

This is probably a valid point. But there is an easy work around: one can run with -Dlicense.skip. Is this not sufficient?
This is also an option for solving your previous concern - to be able to build without license checks delay.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What for ist that? As we allow Java 8 for Hawkular in general, do we need to restrict to 6 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's just for having it easier to contribute some parts of the code to upstream. Is that an acceptable reason?

@tsegismont
Copy link
Contributor

I have no exact numbers. The subjective feeling is that I have not noticed any substantial delay. Should I try to get some exact numbers through running with and without license checks?

It's not necessary, we can revisit that later if needed

This is probably a valid point. But there is an easy work around: one can run with -Dlicense.skip. Is this not sufficient?
This is also an option for solving your previous concern - to be able to build without license checks delay.

That's ok for me. Any objection @pilhuhn ?

Other than that, +1 on merging

@tsegismont
Copy link
Contributor

+1 from @pilhuhn on IRC

tsegismont added a commit that referenced this pull request Jan 20, 2015
Add license header template for com.mycila:license-maven-plugin
@tsegismont tsegismont merged commit 62ebf83 into hawkular:master Jan 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants