gnodet-bot commented on code in PR #13251:
URL: https://github.com/apache/maven/pull/13251#discussion_r4084092889
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java:
##########
@@ -151,6 +151,9 @@ public AbstractUpgradeGoal(StrategyOrchestrator
orchestrator) {
*/
@Override
public int execute(UpgradeContext context) throws Exception {
Review Comment:
⚠️ **Redundancy with subclass overrides.**
`Check.execute()` and `Apply.execute()` both call `setDryRun()` before
invoking `super.execute()`, which overwrites their values with this same
derivation. The two mechanisms are redundant — pick one:
**Option A** (keep only base class): Remove `context.setDryRun(true/false)`
from both subclasses.
**Option B** (keep only subclasses): Remove this line here. The base-class
`execute()` runs after the subclass sets the value, so strategies will always
see the right flag.
Option B is slightly preferable: a future `Goal` subclass can't accidentally
inherit wrong dry-run state because the subclass controls it explicitly.
--
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]
