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/02 09:08:40 UTC

[GitHub] [maven] laeubi opened a new pull request #674: MNG-7407 Allow core-extension to override default CI-Firendly-Versions handling

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


   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-7407) 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 pull request #674: MNG-7407 Allow core-extension to override default CI-Firendly-Versions handling

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


   >     * Why for Maven 3.8.x only? No master?
   
   I can open a PR for master also.
   
   >     * The title is misleading. What you actually did is to move a default implementation to a replace component. @cstamas Did this also in resolver. May be can better rewording this to something like: Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable
   
   Sure do you simply want to rename the PR or should I do so?
   
   >     * Can this be covered by a test somehow?
   
   There are already some tests for this functionality that also run fine with this changes.
   
   


-- 
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 #674: MNG-7407 Allow core-extension to override default CI-Firendly-Versions handling

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



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/DefaultModelVersionProcessor.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.model.interpolation;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Properties;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.model.building.ModelBuildingRequest;
+
+/**
+ * Maven default implementation of the {@link ModelVersionProcessor} to support
+ * <a href="https://maven.apache.org/maven-ci-friendly.html">CI Friendly Versions</a>
+ */
+@Named
+@Singleton
+public class DefaultModelVersionProcessor
+    implements ModelVersionProcessor
+{
+
+    private static final String SHA1_PROPERTY = "sha1";
+
+    private static final String CHANGELIST_PROPERTY = "changelist";
+
+    private static final String REVISION_PROPERTY = "revision";
+
+    @Override
+    public boolean isValidProperty( String property )
+    {
+        return REVISION_PROPERTY.equals( property ) || CHANGELIST_PROPERTY.equals( property )
+            || SHA1_PROPERTY.equals( property );
+    }
+
+    @Override
+    public void overwriteModelProperties( Properties modelProperties, ModelBuildingRequest request )
+    {
+        if ( request.getSystemProperties().containsKey( REVISION_PROPERTY ) )
+        {
+            modelProperties.put( REVISION_PROPERTY, request.getSystemProperties().get( REVISION_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( CHANGELIST_PROPERTY ) )
+        {
+            modelProperties.put( CHANGELIST_PROPERTY, request.getSystemProperties().get( CHANGELIST_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( SHA1_PROPERTY ) )
+        {
+            modelProperties.put( SHA1_PROPERTY, request.getSystemProperties().get( SHA1_PROPERTY ) );

Review comment:
       @rfscholte I believe that this is a bug in our code since those are are supposed to be user properties and not system properties or we need to support both. @laeubi Not a regression from you, but an observation.




-- 
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 #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

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


   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 #674: [3.8.x][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 #674:
URL: https://github.com/apache/maven/pull/674#discussion_r800209514



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/DefaultModelVersionProcessor.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.model.interpolation;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Properties;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.model.building.ModelBuildingRequest;
+
+/**
+ * Maven default implementation of the {@link ModelVersionProcessor} to support
+ * <a href="https://maven.apache.org/maven-ci-friendly.html">CI Friendly Versions</a>
+ */
+@Named
+@Singleton
+public class DefaultModelVersionProcessor
+    implements ModelVersionProcessor
+{
+
+    private static final String SHA1_PROPERTY = "sha1";
+
+    private static final String CHANGELIST_PROPERTY = "changelist";
+
+    private static final String REVISION_PROPERTY = "revision";
+
+    @Override
+    public boolean isValidProperty( String property )
+    {
+        return REVISION_PROPERTY.equals( property ) || CHANGELIST_PROPERTY.equals( property )
+            || SHA1_PROPERTY.equals( property );
+    }
+
+    @Override
+    public void overwriteModelProperties( Properties modelProperties, ModelBuildingRequest request )
+    {
+        if ( request.getSystemProperties().containsKey( REVISION_PROPERTY ) )
+        {
+            modelProperties.put( REVISION_PROPERTY, request.getSystemProperties().get( REVISION_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( CHANGELIST_PROPERTY ) )
+        {
+            modelProperties.put( CHANGELIST_PROPERTY, request.getSystemProperties().get( CHANGELIST_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( SHA1_PROPERTY ) )
+        {
+            modelProperties.put( SHA1_PROPERTY, request.getSystemProperties().get( SHA1_PROPERTY ) );

Review comment:
       Okay I have changed the PR accordingly.




-- 
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 #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

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


   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] michael-o commented on pull request #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

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


   Merged.


-- 
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 edited a comment on pull request #674: MNG-7407 Allow core-extension to override default CI-Firendly-Versions handling

Posted by GitBox <gi...@apache.org>.
michael-o edited a comment on pull request #674:
URL: https://github.com/apache/maven/pull/674#issuecomment-1030866814


   > > ```
   > > * Why for Maven 3.8.x only? No master?
   > > ```
   > 
   > I can open a PR for master also.
   
   Yes, please.
   
   > > ```
   > > * The title is misleading. What you actually did is to move a default implementation to a replace component. @cstamas Did this also in resolver. May be can better rewording this to something like: Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable
   > > ```
   > 
   > Sure do you simply want to rename the PR or should I do so?
   
   Yes, please. PR and JIRA issue.
   
   > > ```
   > > * Can this be covered by a test somehow?
   > > ```
   > 
   > There are already some tests for this functionality that also run fine with this changes.
   
   Please point me to them.


-- 
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 #674: MNG-7407 Allow core-extension to override default CI-Firendly-Versions handling

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



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/DefaultModelVersionProcessor.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.model.interpolation;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Properties;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.model.building.ModelBuildingRequest;
+
+/**
+ * Maven default implementation of the {@link ModelVersionProcessor} to support
+ * <a href="https://maven.apache.org/maven-ci-friendly.html">CI Friendly Versions</a>
+ */
+@Named
+@Singleton
+public class DefaultModelVersionProcessor
+    implements ModelVersionProcessor
+{
+
+    private static final String SHA1_PROPERTY = "sha1";
+
+    private static final String CHANGELIST_PROPERTY = "changelist";
+
+    private static final String REVISION_PROPERTY = "revision";
+
+    @Override
+    public boolean isValidProperty( String property )
+    {
+        return REVISION_PROPERTY.equals( property ) || CHANGELIST_PROPERTY.equals( property )
+            || SHA1_PROPERTY.equals( property );
+    }
+
+    @Override
+    public void overwriteModelProperties( Properties modelProperties, ModelBuildingRequest request )
+    {
+        if ( request.getSystemProperties().containsKey( REVISION_PROPERTY ) )
+        {
+            modelProperties.put( REVISION_PROPERTY, request.getSystemProperties().get( REVISION_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( CHANGELIST_PROPERTY ) )
+        {
+            modelProperties.put( CHANGELIST_PROPERTY, request.getSystemProperties().get( CHANGELIST_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( SHA1_PROPERTY ) )
+        {
+            modelProperties.put( SHA1_PROPERTY, request.getSystemProperties().get( SHA1_PROPERTY ) );

Review comment:
       As far as I understand the idea ias that system properties given on the command line take precedence over everything else. 




-- 
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 #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

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


   > > Please point me to them.
   > 
   > I think this is where it is tested
   > 
   > https://github.com/apache/maven/blob/6ca13af230e9a7c6dcb0bf629d9a2676d8f833ed/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java#L804-L860
   
   Indeed, this should be enough for the default implementation.


-- 
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] rfscholte commented on a change in pull request #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

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



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/DefaultModelVersionProcessor.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.model.interpolation;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Properties;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.model.building.ModelBuildingRequest;
+
+/**
+ * Maven default implementation of the {@link ModelVersionProcessor} to support
+ * <a href="https://maven.apache.org/maven-ci-friendly.html">CI Friendly Versions</a>
+ */
+@Named
+@Singleton
+public class DefaultModelVersionProcessor
+    implements ModelVersionProcessor
+{
+
+    private static final String SHA1_PROPERTY = "sha1";
+
+    private static final String CHANGELIST_PROPERTY = "changelist";
+
+    private static final String REVISION_PROPERTY = "revision";
+
+    @Override
+    public boolean isValidProperty( String property )
+    {
+        return REVISION_PROPERTY.equals( property ) || CHANGELIST_PROPERTY.equals( property )
+            || SHA1_PROPERTY.equals( property );
+    }
+
+    @Override
+    public void overwriteModelProperties( Properties modelProperties, ModelBuildingRequest request )
+    {
+        if ( request.getSystemProperties().containsKey( REVISION_PROPERTY ) )
+        {
+            modelProperties.put( REVISION_PROPERTY, request.getSystemProperties().get( REVISION_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( CHANGELIST_PROPERTY ) )
+        {
+            modelProperties.put( CHANGELIST_PROPERTY, request.getSystemProperties().get( CHANGELIST_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( SHA1_PROPERTY ) )
+        {
+            modelProperties.put( SHA1_PROPERTY, request.getSystemProperties().get( SHA1_PROPERTY ) );

Review comment:
       > @rfscholte I believe that this is a bug in our code since those are are supposed to be user properties and not system properties or we need to support both. 
   
   It should be at least user properties. That's probably enough, as I expect that's the way CI servers can provide a value.
   
   




-- 
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 closed pull request #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

Posted by GitBox <gi...@apache.org>.
michael-o closed pull request #674:
URL: https://github.com/apache/maven/pull/674


   


-- 
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 #674: MNG-7407 Allow core-extension to override default CI-Firendly-Versions handling

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



##########
File path: maven-model-builder/src/main/java/org/apache/maven/model/interpolation/DefaultModelVersionProcessor.java
##########
@@ -0,0 +1,69 @@
+package org.apache.maven.model.interpolation;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.Properties;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.model.building.ModelBuildingRequest;
+
+/**
+ * Maven default implementation of the {@link ModelVersionProcessor} to support
+ * <a href="https://maven.apache.org/maven-ci-friendly.html">CI Friendly Versions</a>
+ */
+@Named
+@Singleton
+public class DefaultModelVersionProcessor
+    implements ModelVersionProcessor
+{
+
+    private static final String SHA1_PROPERTY = "sha1";
+
+    private static final String CHANGELIST_PROPERTY = "changelist";
+
+    private static final String REVISION_PROPERTY = "revision";
+
+    @Override
+    public boolean isValidProperty( String property )
+    {
+        return REVISION_PROPERTY.equals( property ) || CHANGELIST_PROPERTY.equals( property )
+            || SHA1_PROPERTY.equals( property );
+    }
+
+    @Override
+    public void overwriteModelProperties( Properties modelProperties, ModelBuildingRequest request )
+    {
+        if ( request.getSystemProperties().containsKey( REVISION_PROPERTY ) )
+        {
+            modelProperties.put( REVISION_PROPERTY, request.getSystemProperties().get( REVISION_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( CHANGELIST_PROPERTY ) )
+        {
+            modelProperties.put( CHANGELIST_PROPERTY, request.getSystemProperties().get( CHANGELIST_PROPERTY ) );
+        }
+        if ( request.getSystemProperties().containsKey( SHA1_PROPERTY ) )
+        {
+            modelProperties.put( SHA1_PROPERTY, request.getSystemProperties().get( SHA1_PROPERTY ) );

Review comment:
       True, but those are not system properties, those are user properties. A common misconception and badly coined in the Maven core codebase. See: https://github.com/apache/maven/blob/4476026c525a22900c6c5e4f0de6f7362cb201f2/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L1743-L1753




-- 
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 #674: MNG-7407 Allow core-extension to override default CI-Firendly-Versions handling

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


   > > ```
   > > * Why for Maven 3.8.x only? No master?
   > > ```
   > 
   > I can open a PR for master also.
   
   Yes, please.
   
   > > ```
   > > * The title is misleading. What you actually did is to move a default implementation to a replace component. @cstamas Did this also in resolver. May be can better rewording this to something like: Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable
   > > ```
   > 
   > Sure do you simply want to rename the PR or should I do so?
   
   Yes, please. PR and JIRA issue.
   
   
   > > ```
   > > * Can this be covered by a test somehow?
   > > ```
   > 
   > There are already some tests for this functionality that also run fine with this changes.
   
   


-- 
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 #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

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


   Title updated as you suggested.
   
   


-- 
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 #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

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


   > > I can open a PR for master also.
   > 
   > Yes, please.
   
   Done: 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 pull request #674: MNG-7407 Allow core-extension to override default CI-Firendly-Versions handling

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


   > Please point me to them.
   
   I think this is where it is tested https://github.com/apache/maven/blob/6ca13af230e9a7c6dcb0bf629d9a2676d8f833ed/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java#L804-L860


-- 
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 #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

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


   > 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 #674: [3.8.x][MNG-7407] Introduce a ModelVersionProcessor component to make CI Friends Versions pluggable

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


   > 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