Warn when the aggregate goal skips a non-pom project - #325
Open
kratos0718 wants to merge 1 commit into
Open
Conversation
AggregatorSourceJarMojo.doExecute() packaged sources only for "pom" packaging and returned silently otherwise, so a project with an incidental aggregator binding produced no source JAR and no indication why. Add an else branch logging a warning that names the goal, the required packaging and the project's actual packaging. Behaviour is otherwise unchanged: the build still succeeds, so no currently-passing build turns red. The goal name is now a constant shared by the @mojo annotation and the message so the two cannot drift apart.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #306.
Problem
AggregatorSourceJarMojo.doExecute()packages sources only when the project usespompackaging:With any other packaging it returns silently. No source JAR is produced and the user gets no indication why — which, as the issue notes, is a plausible outcome of a misleading build or configuration even though
@Mojo(aggregator = true)normally impliespom.Change
Adds an
elsebranch that logs a warning naming the goal, the required packaging, and the project's actual packaging:The goal name is now a constant shared by the
@Mojoannotation and the message, so the two cannot drift apart.On warning vs failing
@elharo asked in the issue whether this should fail rather than log. I went with a warning, for the reason discussed there: a non-pom aggregator run succeeds today, so failing would turn currently-green builds red without any change on the user's side, and the people most likely to hit it are exactly those with an incidental binding they never noticed. The complaint in the issue is diagnosability, which the warning resolves fully.
If the project would rather treat it as an outright misconfiguration, I am happy to switch this to a failure — I would just suggest tying that to a major version. Say the word and I will push the change.
Tests
AggregatorSourceJarMojoTestadds two cases:warnsWhenPackagingIsNotPom— asserts the warning is emitted and contains the goal, the required packaging and the actual packagingdoesNotWarnWhenPackagingIsPom— asserts no warning on a legitimatepomrun, so the change cannot over-correct into warning on valid buildsBehaviour for
pompackaging is unchanged.