dhruv9b commented on PR #1022:
URL: https://github.com/apache/maven-enforcer/pull/1022#issuecomment-5821155957
> Thanks for working on this! A few remarks before we can merge.
>
> ### 1. The notation should be consistent with `dependency:tree`
> Maven already has an established notation for a managed version —
`maven-dependency-tree` (`VerboseDependencyNode`) renders it in the verbose
dependency tree as:
>
> ```
> +- org.example:childA:jar:1.0.0:compile (version managed from 2.0.0)
> ```
>
> Rather than introducing a third variant, let's reuse exactly that, so a
user sees the same wording in `mvn dependency:tree -Dverbose` and in the
enforcer output:
>
> ```
> +-org.example:childA:1.0.0 (version managed from 2.0.0)
> ```
>
> This affects the rule, the unit test and
`require-upper-bound-dependencies-managed_failure/verify.groovy`.
>
> ### 2. Please add a new test instead of modifying the existing one
> `RequireUpperBoundDepsTest#testRule` currently covers the plain conflict
between two dependency paths and asserts that both versions show up in the
message. After adding `withPremanagedVersion("2.0.0")` to the first child, that
node alone already triggers the conflict and the assertion no longer verifies
the two-path output — so the original case is effectively no longer covered.
>
> Could you please restore `testRule` to its previous form and add a
separate test (e.g. `testManagedVersion`) for the managed case? Then both
behaviours stay covered. `DependencyNodeBuilder#withPremanagedVersion` is a
good addition and can stay as is.
>
> ### 3. Documentation text is placed inside the code block
> In `enforcer-rules/src/site/markdown/requireUpperBoundDeps.md.vm` the new
sentence was inserted before the closing fence, so it will be rendered as part
of the sample log output. It has to go after the closing `` ``` ``.
>
> Also, the sample log in that block contains no managed dependency at all,
so the explanation has nothing to refer to. A short, separate example showing a
managed dependency would be much clearer.
>
> ### 4. Unrelated changes
> `buildErrorMessage()` only gained blank lines — please drop them to keep
the diff focused on one change.
My bad I was busy last week,I have changed the things as you asked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
