Jasig / maven-notice-plugin

Apache License 2.0
7 stars 16 forks source link

Added parameter to skip running NOTICE checks. #2

Closed mmoayyed closed 11 years ago

mmoayyed commented 11 years ago

In multi-module Maven projects, declaring the notice plugin at the root parent pom causes the plugin to be invoked every time for every module. This is specially annoying if NOTICE files are up to date and correct (checked manually by directly invoking the plugin from the command prompt) as there would be no need for a second verification. Additionally, the plugin attempts to repetitively check for modules that have already been validated.

If the multi-module build includes 3 modules of A, B, C, running the build with the plugin declared will validate NOTICE files for A, then A and B, then A and B and C. Obviously, not ideal.

The validation process is particularly slow, and at times may cause out-of-memory issues specially during releasing of larger maven projects, such as CAS.

battags commented 11 years ago

I'd like to see this change make it in.

mmoayyed commented 11 years ago

Any objections to cutting the next release for the plugin with this change in place?

battags commented 11 years ago

Was the release cut?

mmoayyed commented 11 years ago

Not yet. I was sort of waiting for +1s. I’ll see if I can cut a release towards the end of the week.

battags commented 11 years ago

+1 +1 +1 +1 :-)

mmoayyed commented 11 years ago

:))

apetro commented 11 years ago

Why is adding a parameter to skip running NOTICE checks a good idea, and could that why be articulated in the description of this pull request?

mmoayyed commented 11 years ago

Added

apetro commented 11 years ago

Is there an opportunity for statefulness within the plugin during a given build process to keep track of what it has already verified and remember to fast-forward through subsequent attempts to verify during that build run?

parameter allowing disabling this annoyance to allow builds to go forward is good. enhancement so that it runs sufficiently efficiently that it doesn't need to be disabled better?

wgthom commented 11 years ago

On Thu, Aug 15, 2013 at 3:44 PM, Andrew Petro notifications@github.comwrote:

Is there an opportunity for statefulness within the plugin during a given build process to keep track of what it has already verified and remember to fast-forward through subsequent attempts to verify during that build run?

Yes, isn't that called gradle? ;)

parameter allowing disabling this annoyance to allow builds to go forward is good. enhancement so that it runs sufficiently efficiently that it doesn't need to be disabled better?

— Reply to this email directly or view it on GitHubhttps://github.com/Jasig/maven-notice-plugin/pull/2#issuecomment-22724721 .

mmoayyed commented 11 years ago

The sad news is, I cant actually cut the release with the repo as it stands. There are files in the test-repo directory with a ":" in the name that some platforms consider to be invalid file names!

battags commented 11 years ago

Any update to this?

mmoayyed commented 11 years ago

@battags sorry for the delay. yes, I cut the 1.0.6 release yesterday. should be available to maven central by now.

battags commented 11 years ago

Awesome, thank you!

battags commented 10 years ago

I'm finding the following does not work: "mvn release:prepare -Dskip.checks=true" Am I doing something wrong?

mmoayyed commented 10 years ago

You are not. The release plugin forks the VM so -D parameters aren't passed on. You have to pass parameters to the forked VM (which I could never get to work) with a -Darguments=... or set the property in your settings.xml file which worked for me.

battags commented 10 years ago

Just a note, this is why we can't just pass it in via arguments: http://stackoverflow.com/questions/4660047/override-maven-plugin-configuration-defined-in-pluginmanagement-from-the-command

We should potentially release a version 37 of the parent pom which lets us pass in a value to be included in the release arguments. Thoughts?

mmoayyed commented 10 years ago

Sounds good. Lets.

battags commented 10 years ago

I'm running the following command (Jasig Parent POM 37): /development/apache-maven-3.1.1/bin/mvn release:prepare -DadditionalReleaseArguments="-Dskip.checks=true"

which translates into: Executing: /bin/sh -c cd /Users/battags/git-projects/jasig-java-cas-client && /development/apache-maven-3.1.1/bin/mvn clean verify --no-plugin-updates -Dskip.checks=true -Psonatype-oss-release,jasig-release

which seems to still be failing. Any clues to where this is going wrong? I feel like this is way harder than it should be ;-)

mmoayyed commented 10 years ago

Hmmm...I wonder if the fact that the field is assigned a value of false has something to do with the command line parameter not having any effect?

private boolean skipChecks = false;

battags commented 10 years ago

I would imagine it shouldn't (since it would have been false anyway) plus you said it worked for you from the settings.xml ?

mmoayyed commented 10 years ago

It does, yes. Strange. Let me work on this a little bit from the command-line this time. I'll post back to this thread in a bit.

mmoayyed commented 10 years ago

Turns out the parameter definition should actually be:

@parameter expression="${skip.checks}"

Now, the question is; why does @parameter expression="skip.checks" work in the xml file and not from the command-line?!!

I can cut a 1.0.6.1 release with that change committed.

battags commented 10 years ago

and I can cut a jasig parent pom 38 tonight :-) Thanks for debugging!

mmoayyed commented 10 years ago

This is now done.

I also created a trunk branch with the intention of using it purely for development. master would be used for releases. This is done, so master would be able ignore/remove certain files that are considered invalid during a checkout. Makes it a whole lot easier to cut releases on master without having to worry about files with names like "hello:world:1:2:3".

mmoayyed commented 10 years ago

btw, @battags, it may also be worth updating the cas pom itself to point to the latest parent version.