Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#794: SetMojo would always change the version of the POM, regardless if a match was found #799

Merged

Conversation

andrzejj0
Copy link
Contributor

@andrzejj0 andrzejj0 commented Oct 26, 2022

Probably an old technical debt of sorts in SetMojo: the mojo would normally change the files matching oldVersion, groupId, or ArtifactId.

However, if no match was found in the reactor, then the root model is changed anyway!

https://github.com/mojohaus/versions-maven-plugin/blob/b75a5656113f8974e7bb26f2cd0cdeacff59800e/src/main/java/org/codehaus/mojo/versions/SetMojo.java#L384

I don't understand the reason for it. When checked git blame, the original author added those lines to fix some breaking integration tests. However, it looks like no tests fail even with those lines removed.

Anyhow. I don't think that behaviour was intuitive and it is certainly very unexpected.

A side effect of that behaviour was that, with "always", the timestamp of the file was updated regardless if a match was found. So I had to explicitly add all reactor files if the update timestamp switch is set to "always".

@andrzejj0 andrzejj0 force-pushed the issue-794-set-old-version branch from 9ea8fb0 to 20a994b Compare October 26, 2022 17:44
@andrzejj0
Copy link
Contributor Author

@slawekjaranowski Please review and merge.

@andrzejj0
Copy link
Contributor Author

Please merge unless you have any more remarks.

@andrzejj0 andrzejj0 force-pushed the issue-794-set-old-version branch from 20a994b to e0b9e68 Compare November 3, 2022 05:55
@andrzejj0
Copy link
Contributor Author

Rebased

@slawekjaranowski slawekjaranowski added this to the 2.14.0 milestone Nov 3, 2022
@slawekjaranowski slawekjaranowski merged commit 8c13de2 into mojohaus:master Nov 3, 2022
@andrzejj0 andrzejj0 deleted the issue-794-set-old-version branch November 3, 2022 13:31
andrzejj0 added a commit to andrzejj0/versions-maven-plugin that referenced this pull request Feb 25, 2023
- Reviewed the code and the semantics of processAllModules, groupId, artifactId, oldVersion params
- Dropped support for partially interpolating the properties to be matched with groupId:artifactId:oldVersion
- Matching occurs always unless *processAllModules* is set
- If *processAllModules* is not set, *oldVersion* will be respected
andrzejj0 added a commit to andrzejj0/versions-maven-plugin that referenced this pull request Mar 14, 2023
- Reviewed the code and the semantics of processAllModules, groupId, artifactId, oldVersion params
- Dropped support for partially interpolating the properties to be matched with groupId:artifactId:oldVersion
- Matching occurs always unless *processAllModules* is set
- If *processAllModules* is not set, *oldVersion* will be respected
- Included an additional test case from mojohaus#916 testing child version
andrzejj0 added a commit to andrzejj0/versions-maven-plugin that referenced this pull request May 10, 2023
- Reviewed the code and the semantics of processAllModules, groupId, artifactId, oldVersion params
- Dropped support for partially interpolating the properties to be matched with groupId:artifactId:oldVersion
- Matching occurs always unless *processAllModules* is set
- If *processAllModules* is not set, *oldVersion* will be respected
- Included an additional test case from mojohaus#916 testing child version
slawekjaranowski pushed a commit that referenced this pull request May 14, 2023
- Reviewed the code and the semantics of processAllModules, groupId, artifactId, oldVersion params
- Dropped support for partially interpolating the properties to be matched with groupId:artifactId:oldVersion
- Matching occurs always unless *processAllModules* is set
- If *processAllModules* is not set, *oldVersion* will be respected
- Included an additional test case from #916 testing child version
dongjoon-hyun pushed a commit to apache/spark that referenced this pull request Jun 18, 2023
### What changes were proposed in this pull request?
The pr aims to update some maven plugins to newest version. include:
- versions-maven-plugin from 2.15.0 to 2.16.0
- maven-source-plugin from 3.2.1 to 3.3.0
- maven-surefire-plugin from 3.1.0 to 3.1.2
- maven-dependency-plugin from 3.5.0 to 3.6.0

### Why are the changes needed?
- versions-maven-plugin
1.Release Notes: https://github.com/mojohaus/versions/releases/tag/2.16.0
2.Bug Fix:
Resolves: display-dependency-updates only shows updates from the most major allowed segment (mojohaus/versions#966) ajarmoniuk
Resolves mojohaus/versions#931: Fixing problems with encoding in UseDepVersion and PomHelper (mojohaus/versions#932) ajarmoniuk
Resolves mojohaus/versions#916: Partially reverted mojohaus/versions#799. (mojohaus/versions#924) ajarmoniuk
Resolves mojohaus/versions#954: Excluded plexus-container-default (mojohaus/versions#955) ajarmoniuk
Resolves mojohaus/versions#951: DefaultArtifactVersion::getVersion can be null (mojohaus/versions#952) ajarmoniuk
BoundArtifactVersion.toString() to work with NumericVersionComparator (mojohaus/versions#930) ajarmoniuk
Issue mojohaus/versions#925: Protect against an NPE if a dependency version is defined in dependencyManagement (mojohaus/versions#926) ajarmoniuk

- maven-source-plugin
v3.2.1 VS v3.3.0: apache/maven-source-plugin@maven-source-plugin-3.2.1...maven-source-plugin-3.3.0

- maven-surefire-plugin
Release Notes: https://github.com/apache/maven-surefire/releases/tag/surefire-3.1.2

- maven-dependency-plugin
v3.5.0 VS v3.6.0: apache/maven-dependency-plugin@maven-dependency-plugin-3.5.0...maven-dependency-plugin-3.6.0

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA.

Closes #41641 from panbingkun/SPARK-44085.

Authored-by: panbingkun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
czxm pushed a commit to czxm/spark that referenced this pull request Jun 19, 2023
### What changes were proposed in this pull request?
The pr aims to update some maven plugins to newest version. include:
- versions-maven-plugin from 2.15.0 to 2.16.0
- maven-source-plugin from 3.2.1 to 3.3.0
- maven-surefire-plugin from 3.1.0 to 3.1.2
- maven-dependency-plugin from 3.5.0 to 3.6.0

### Why are the changes needed?
- versions-maven-plugin
1.Release Notes: https://github.com/mojohaus/versions/releases/tag/2.16.0
2.Bug Fix:
Resolves: display-dependency-updates only shows updates from the most major allowed segment (mojohaus/versions#966) ajarmoniuk
Resolves mojohaus/versions#931: Fixing problems with encoding in UseDepVersion and PomHelper (mojohaus/versions#932) ajarmoniuk
Resolves mojohaus/versions#916: Partially reverted mojohaus/versions#799. (mojohaus/versions#924) ajarmoniuk
Resolves mojohaus/versions#954: Excluded plexus-container-default (mojohaus/versions#955) ajarmoniuk
Resolves mojohaus/versions#951: DefaultArtifactVersion::getVersion can be null (mojohaus/versions#952) ajarmoniuk
BoundArtifactVersion.toString() to work with NumericVersionComparator (mojohaus/versions#930) ajarmoniuk
Issue mojohaus/versions#925: Protect against an NPE if a dependency version is defined in dependencyManagement (mojohaus/versions#926) ajarmoniuk

- maven-source-plugin
v3.2.1 VS v3.3.0: apache/maven-source-plugin@maven-source-plugin-3.2.1...maven-source-plugin-3.3.0

- maven-surefire-plugin
Release Notes: https://github.com/apache/maven-surefire/releases/tag/surefire-3.1.2

- maven-dependency-plugin
v3.5.0 VS v3.6.0: apache/maven-dependency-plugin@maven-dependency-plugin-3.5.0...maven-dependency-plugin-3.6.0

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA.

Closes apache#41641 from panbingkun/SPARK-44085.

Authored-by: panbingkun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants