search

alexo / wro4j

New project location is:https://github.com/wro4j/wro4j
442 stars 110 forks source link

Break build when resource is invalid and ignoreMissingResources = false #208

Closed muffl0n closed 10 years ago

muffl0n commented 10 years ago

Somehow the incrementalBuilding-Feature broke the behaviour that ignoreMissingResource should provide. This PR throws a MojoExecutionException if a resource is invalid and ignoreMissingResource is set to false.

muffl0n commented 10 years ago

Followup to #206

muffl0n commented 10 years ago

The exception is thrown if the resource is invalid. E.g. if it can't be read or does not exist. The plugin should then obey the setting the user made for "ignoreMissingResources". While deploying our webapp to our production servers we wan't to make sure that there is none of the configured resources missing. The maven build should fail an so should the whole job in Jenkins. This PR enables us to do so. It gives the setting " ignoreMissingResources" back it's meaning.

muffl0n commented 10 years ago

In other words: what IOException could occure that is only fatal to the change detection but not to the rest of the plugins execution? If scanning for changes fails, all other operations in this file will most likely fail, too.

alexo commented 10 years ago

If there is a missing resource, the build should obey the "ignoreMissingResources" meaning. But, the exception will be thrown during processing. There is no need to duplicate this check in resource change detection.

alexo commented 10 years ago

In other words: what IOException could occure that is only fatal to the change detection but not to the rest of the plugins execution?

I agree with that statement. The only thing that bothers me, is duplicate check for 'ignoreMissingResources' (here and in processor exception handling).

muffl0n commented 10 years ago

I agree that the check for ignoreMissingResources should not be duplicated. I will see if I can check why it didn't work for me in the first place tomorrow at the office.

muffl0n commented 10 years ago

That's weird. I can't reproduce the false positive I got yesterday. It didn't matter if I set ignoreMissingResources to false, the build did not break as intended. So I created this PR which should just be ignored now.

Sorry for the inconvenience.

alexo commented 10 years ago

It is not excluded that there is a bug hiding somewhere... There is nothing wrong with this PR. I could merge it... the only thing missing, is an unit test proving it works as intended.

muffl0n commented 10 years ago

If there really is a bug somewhere I prefer to hunt that one instead to patch the problem at another position. I will try to reproduce this bug and see if I can fix it.

alexo commented 10 years ago

@muffl0n sound reasonable. Thanks!