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

[MNG-8230] Rewrite CI friendly versions #1710

Merged
merged 2 commits into from
Oct 1, 2024
Merged

Conversation

gnodet
Copy link
Contributor

@gnodet gnodet commented Sep 9, 2024

CI Friendly versions are now fully opened, so not limited anymore to the 3 predefined properties which were sha1, changelist and revision.
So expression on version and parent version are interpolated with their proper values, taken from the session user properties and from the session's root project properties (uninterpolated).
An additional validation check after computing this file model to ensure that no expression is used in version at this point. This ensures that during the next step (raw model build), all the full GAV are known, as they will be used to infer missing dependency information. It allows remove possible problems with plugins, not being fully aware of those versions.


JIRA issue: https://issues.apache.org/jira/browse/MNG-8230
IT PR: apache/maven-integration-testing#366

@gnodet gnodet added this to the 4.0.0-beta-5 milestone Sep 9, 2024
@gnodet gnodet requested a review from cstamas September 9, 2024 14:51
@gnodet gnodet force-pushed the MNG-8230 branch 6 times, most recently from 668da63 to 970bf26 Compare September 17, 2024 15:33
Copy link
Member

@cstamas cstamas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR seems mixed with DI?

@gnodet
Copy link
Contributor Author

gnodet commented Sep 18, 2024

This PR seems mixed with DI?

Yes, it requires a session scope which is not well supported by the sisu/di bridge. I'll rebase once #1722 is merged.

@gnodet gnodet force-pushed the MNG-8230 branch 2 times, most recently from 8685029 to 3581881 Compare September 18, 2024 15:48
@cstamas
Copy link
Member

cstamas commented Sep 18, 2024

IMO, the proper way of working for CFV is:

  • any placeholder can be used (OK)
  • as long it has value defined, but this part is NOT OK, at least I see no code that makes sure version is not left placeholder?

Or I miss something?

@gnodet
Copy link
Contributor Author

gnodet commented Sep 18, 2024

IMO, the proper way of working for CFV is:

  • any placeholder can be used (OK)
  • as long it has value defined, but this part is NOT OK, at least I see no code that makes sure version is not left placeholder?

Or I miss something?

This role is supposed to be hold by the ModelValidator ...

@gnodet gnodet marked this pull request as draft September 19, 2024 20:16
@gnodet gnodet force-pushed the MNG-8230 branch 2 times, most recently from fcf7337 to 4805dbe Compare September 30, 2024 09:14
@gnodet gnodet marked this pull request as ready for review September 30, 2024 11:43
@gnodet gnodet merged commit 885a4b3 into apache:master Oct 1, 2024
13 checks passed
@gnodet gnodet deleted the MNG-8230 branch October 2, 2024 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants