[MNG-6207] display build WARNING if deprecated 'system' scope is used #119
Conversation
| @@ -604,17 +609,21 @@ public void testBadImportScopeClassifier() | |||
| "'dependencyManagement.dependencies.dependency.classifier' for test:a:pom:cls must be empty" ); | |||
| } | |||
|
|
|||
| public void testSystemPathRefersToProjectBasedir() | |||
| public void testSystemPathRefersToProjectBasedirAndDeprecatedScope() | |||
There was a problem hiding this comment.
I think it's better having different test cases. Keeping the old test cases and create new ones for the changed behaviour. That makes it safe not to break previous behaviour,
| @@ -411,14 +411,19 @@ public void testIncompleteParent() | |||
| assertTrue( result.getFatals().get( 2 ).contains( "parent.version" ) ); | |||
| } | |||
|
|
|||
| public void testHardCodedSystemPath() | |||
| public void testHardCodedSystemPathAndDeprecatedScope() | |||
There was a problem hiding this comment.
I think it's better having different test cases. Keeping the old test cases and create new ones for the changed behaviour. That makes it safe not to break previous behaviour,
|
New changes are pushed via new commit (all commits will be squashed into one eventually). Test renaming is recalled; however, it seems kind of reasonable (necessary ?) to fine-grain those test cases instead of adding a new one.
it seems convenient to carefully reuse those existing test cases, because both tags end up tested anyway (I reckon that adding a new test case with same two tags would be redundant). @khmarbaise: let me know what you think (and please forgive me if my rationale is not that rational, I'll be ready to dig deeper and accommodate solution in order to fit standards). Bonus question (in the unlikely case that I got it all right): does it make sense to introduce new ModelBuildingRequest VALIDATION_LEVEL_MAVEN (3_5 or 3_6) ? |
|
Just to narrow discussion to PR scope only and to eliminate bonus question: there is/was an initiative to increase validation level (MNG-6075 Increase the model validation level to the next minor level version). |
|
Could you make a real commit log message like: Otherwise I need to do a |
dejan2609
force-pushed
the
MNG-6207-system-scope-deprecation-warning
branch
from
59ab313
to
7ce4ab9
Compare
|
@khmarbaise done, I altered commit messaged (added note about tests). |
|
Sorry...Can please make the PR new based on the current master of the apache-maven, cause I didn't realized that this PR must be done before the other...Thanks in advance... |
…ope 'system' declaration Note about tests: existing 'systemPath' related tests are reused/expanded (rationale: scope 'system' and 'systemPath' are mutually dependent)
dejan2609
force-pushed
the
MNG-6207-system-scope-deprecation-warning
branch
from
7ce4ab9
to
1ddb1f3
Compare
|
@khmarbaise Ok, I'll open new PR then. |
JIRA ticket: MNG-6207: Create WARNINGs in case of using system scope
Note: since system scope requires systemPath I decided to expand and rename existing systemPath tests; if that kind of change is not acceptable please suggest different approach.