You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/02/06 18:24:24 UTC

[GitHub] [maven] laeubi opened a new pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

laeubi opened a new pull request #675:
URL: https://github.com/apache/maven/pull/675


   Currently the handling for https://maven.apache.org/maven-ci-friendly.html is hard wired into maven.
   
   For Tycho we like to replace/extend the default handling for this to supply the user with some automatic derived values for some of the variables.
   
   This PR extracts the parts of that handling in a new ModelVersionProcessor that has the current behavior as a default implemented. This not only makes it more clear how this is handled (no cross references between DefaultModelValidator and AbstractStringBasedModelInterpolator required) but also allows to override the handling by a core extension.
   
   FYI @mickaelistria @akurtakov
   
   Following this checklist to help us incorporate your
   contribution quickly and easily:
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
          for the change (usually before you start working on it).  Trivial changes like typos do not
          require a JIRA issue. Your pull request should address just this issue, without
          pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] Format the pull request title like `[MNG-XXX] SUMMARY`, where you replace `MNG-XXX`
          and `SUMMARY` with the appropriate JIRA issue. Best practice is to use the JIRA issue
          title in the pull request title and in the first line of the commit message.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
          be performed on your pull request automatically.
    - [ ] You have run the [Core IT][core-its] successfully.
   
   If your pull request is about ~20 lines of code you don't need to sign an
   [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
   please ask on the developers list.
   
   To make clear that you license your contribution under
   the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   you have to acknowledge this by using the following check-box.
   
    - [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   
    - [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   [core-its]: https://maven.apache.org/core-its/core-it-suite/
   


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] laeubi commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
laeubi commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805395058



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -85,13 +79,22 @@
     private final PathTranslator pathTranslator;
     private final UrlNormalizer urlNormalizer;
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;
+
     @Inject
     public AbstractStringBasedModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
     {
         this.pathTranslator = pathTranslator;
         this.urlNormalizer = urlNormalizer;
     }
 
+    public AbstractStringBasedModelInterpolator setVersionPropertiesProcessor( ModelVersionProcessor processor )

Review comment:
       Done.




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] michael-o commented on pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #675:
URL: https://github.com/apache/maven/pull/675#issuecomment-1038007148


   Please rebase


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] laeubi commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
laeubi commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805399934



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/StringSearchModelInterpolator.java
##########
@@ -68,14 +68,15 @@
     }
 
     @Inject
-    public StringSearchModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
+    public StringSearchModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer,
+                                          ModelVersionProcessor processor )
     {
-        super( pathTranslator, urlNormalizer );
+        super( pathTranslator, urlNormalizer, processor );
     }
 
     StringSearchModelInterpolator()
     {
-        super( null, null );
+        super( null, null, null );

Review comment:
       I think that was for test only but is no longer used, should I remove that?




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] michael-o commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805399775



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/StringSearchModelInterpolator.java
##########
@@ -68,14 +68,15 @@
     }
 
     @Inject
-    public StringSearchModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
+    public StringSearchModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer,
+                                          ModelVersionProcessor processor )
     {
-        super( pathTranslator, urlNormalizer );
+        super( pathTranslator, urlNormalizer, processor );
     }
 
     StringSearchModelInterpolator()
     {
-        super( null, null );
+        super( null, null, null );

Review comment:
       @cstamas Can you come up with a reason why we have this?




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] michael-o commented on pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #675:
URL: https://github.com/apache/maven/pull/675#issuecomment-1042622422


   > > The bug can be worked around for your PR.
   > 
   > Anything I could do in the meantime to mitigate this?
   
   Just need a bit of time. It is almost complete on my side.


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] michael-o commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805394210



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -85,13 +79,22 @@
     private final PathTranslator pathTranslator;
     private final UrlNormalizer urlNormalizer;
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;
+
     @Inject
     public AbstractStringBasedModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
     {
         this.pathTranslator = pathTranslator;
         this.urlNormalizer = urlNormalizer;
     }
 
+    public AbstractStringBasedModelInterpolator setVersionPropertiesProcessor( ModelVersionProcessor processor )

Review comment:
       Please switch to ctor injection, this will give you two benefits:
   1. All required objects are wired and are not optional
   2. You can wire manually for testing if necessary, opposite of field injection




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] laeubi commented on pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
laeubi commented on pull request #675:
URL: https://github.com/apache/maven/pull/675#issuecomment-1038171004


   > Please rebase
   
   Done!


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] laeubi commented on pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
laeubi commented on pull request #675:
URL: https://github.com/apache/maven/pull/675#issuecomment-1038171004


   > Please rebase
   
   Done!


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] michael-o commented on pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #675:
URL: https://github.com/apache/maven/pull/675#issuecomment-1038441137


   So I have good and bad news. This change revealed a really really ugly bug in Maven. I will share details tomorrow and need to discuss this with @rfscholte. Good news: The bug can be worked around for your PR.


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] asfgit closed pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #675:
URL: https://github.com/apache/maven/pull/675


   


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] laeubi commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
laeubi commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805399492



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
##########
@@ -89,6 +85,15 @@
 
     private final Set<String> validProfileIds = new HashSet<>();
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;

Review comment:
       Changed to ctr injection




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] laeubi commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
laeubi commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805393804



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -85,13 +79,22 @@
     private final PathTranslator pathTranslator;
     private final UrlNormalizer urlNormalizer;
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;
+
     @Inject
     public AbstractStringBasedModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
     {
         this.pathTranslator = pathTranslator;
         this.urlNormalizer = urlNormalizer;
     }
 
+    public AbstractStringBasedModelInterpolator setVersionPropertiesProcessor( ModelVersionProcessor processor )

Review comment:
       it seems that the tests do not use injection but require to set injected items, e.g. [DefaultModelValidator.java](https://github.com/apache/maven/pull/675/files/614a44f28924cc2bdfb6c668d0004c2086da5267#diff-690df442061d3325c6a7cf7b687e90ae79dd3639d45b8d1e606ad1bbc97a00e2) uses always fields for injection and as an Abstract class it seems a bit strange to inject via ctro




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] laeubi commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
laeubi commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805393804



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -85,13 +79,22 @@
     private final PathTranslator pathTranslator;
     private final UrlNormalizer urlNormalizer;
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;
+
     @Inject
     public AbstractStringBasedModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
     {
         this.pathTranslator = pathTranslator;
         this.urlNormalizer = urlNormalizer;
     }
 
+    public AbstractStringBasedModelInterpolator setVersionPropertiesProcessor( ModelVersionProcessor processor )

Review comment:
       it seems that the tests do not use injection but require to set injected items, e.g. [DefaultModelValidator.java](https://github.com/apache/maven/pull/675/files/614a44f28924cc2bdfb6c668d0004c2086da5267#diff-690df442061d3325c6a7cf7b687e90ae79dd3639d45b8d1e606ad1bbc97a00e2) uses always fields for injection and as an Abstract class it seems a bit strange to inject via ctro

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -85,13 +79,22 @@
     private final PathTranslator pathTranslator;
     private final UrlNormalizer urlNormalizer;
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;
+
     @Inject
     public AbstractStringBasedModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
     {
         this.pathTranslator = pathTranslator;
         this.urlNormalizer = urlNormalizer;
     }
 
+    public AbstractStringBasedModelInterpolator setVersionPropertiesProcessor( ModelVersionProcessor processor )

Review comment:
       Done.

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -86,10 +80,15 @@
     private final UrlNormalizer urlNormalizer;
 
     @Inject

Review comment:
       Done.

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
##########
@@ -89,6 +85,15 @@
 
     private final Set<String> validProfileIds = new HashSet<>();
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;

Review comment:
       Changed to ctr injection

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/StringSearchModelInterpolator.java
##########
@@ -68,14 +68,15 @@
     }
 
     @Inject
-    public StringSearchModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
+    public StringSearchModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer,
+                                          ModelVersionProcessor processor )
     {
-        super( pathTranslator, urlNormalizer );
+        super( pathTranslator, urlNormalizer, processor );
     }
 
     StringSearchModelInterpolator()
     {
-        super( null, null );
+        super( null, null, null );

Review comment:
       I think that was for test only but is no longer used, should I remove that?




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] laeubi commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
laeubi commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805399355



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -86,10 +80,15 @@
     private final UrlNormalizer urlNormalizer;
 
     @Inject

Review comment:
       Done.




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] laeubi commented on pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
laeubi commented on pull request #675:
URL: https://github.com/apache/maven/pull/675#issuecomment-1042612293


   > The bug can be worked around for your PR.
   
   Anything I could do in the meantime to mitigate this?


-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] michael-o commented on pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #675:
URL: https://github.com/apache/maven/pull/675#issuecomment-1038007148






-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] michael-o commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805397271



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
##########
@@ -89,6 +85,15 @@
 
     private final Set<String> validProfileIds = new HashSet<>();
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;

Review comment:
       Any reason why this is not on ctor injection?

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -86,10 +80,15 @@
     private final UrlNormalizer urlNormalizer;
 
     @Inject

Review comment:
       This `@Inject` is obsolete now. Move the new field up to the other private finals.




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] michael-o commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805389880



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -85,13 +79,22 @@
     private final PathTranslator pathTranslator;
     private final UrlNormalizer urlNormalizer;
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;
+
     @Inject
     public AbstractStringBasedModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
     {
         this.pathTranslator = pathTranslator;
         this.urlNormalizer = urlNormalizer;
     }
 
+    public AbstractStringBasedModelInterpolator setVersionPropertiesProcessor( ModelVersionProcessor processor )

Review comment:
       I do not fully understand why this is a setter, but the private field is injected. Why not inject on directly on the ctor? At the end this is a required field not optional, no?

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -85,13 +79,22 @@
     private final PathTranslator pathTranslator;
     private final UrlNormalizer urlNormalizer;
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;
+
     @Inject
     public AbstractStringBasedModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
     {
         this.pathTranslator = pathTranslator;
         this.urlNormalizer = urlNormalizer;
     }
 
+    public AbstractStringBasedModelInterpolator setVersionPropertiesProcessor( ModelVersionProcessor processor )

Review comment:
       Please switch to ctor injection, this will give you two benefits:
   1. All required objects are wired and are not optional
   2. You can wire manually for testing if necessary, opposite of field injection

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
##########
@@ -89,6 +85,15 @@
 
     private final Set<String> validProfileIds = new HashSet<>();
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;

Review comment:
       Any reason why this is not on ctor injection?

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -86,10 +80,15 @@
     private final UrlNormalizer urlNormalizer;
 
     @Inject

Review comment:
       This `@Inject` is obsolete now. Move the new field up to the other private finals.

##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/StringSearchModelInterpolator.java
##########
@@ -68,14 +68,15 @@
     }
 
     @Inject
-    public StringSearchModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
+    public StringSearchModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer,
+                                          ModelVersionProcessor processor )
     {
-        super( pathTranslator, urlNormalizer );
+        super( pathTranslator, urlNormalizer, processor );
     }
 
     StringSearchModelInterpolator()
     {
-        super( null, null );
+        super( null, null, null );

Review comment:
       @cstamas Can you come up with a reason why we have this?




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven] michael-o commented on a change in pull request #675: [MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #675:
URL: https://github.com/apache/maven/pull/675#discussion_r805389880



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
##########
@@ -85,13 +79,22 @@
     private final PathTranslator pathTranslator;
     private final UrlNormalizer urlNormalizer;
 
+    @Inject
+    private ModelVersionProcessor versionProcessor;
+
     @Inject
     public AbstractStringBasedModelInterpolator( PathTranslator pathTranslator, UrlNormalizer urlNormalizer )
     {
         this.pathTranslator = pathTranslator;
         this.urlNormalizer = urlNormalizer;
     }
 
+    public AbstractStringBasedModelInterpolator setVersionPropertiesProcessor( ModelVersionProcessor processor )

Review comment:
       I do not fully understand why this is a setter, but the private field is injected. Why not inject on directly on the ctor? At the end this is a required field not optional, no?




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org