Skip to content

[pom] Use ${revision} replace specific version.#2623

Merged
whhe merged 1 commit intoapache:masterfrom
joyCurry30:use_revision
Nov 6, 2023
Merged

[pom] Use ${revision} replace specific version.#2623
whhe merged 1 commit intoapache:masterfrom
joyCurry30:use_revision

Conversation

@joyCurry30
Copy link
Copy Markdown
Contributor

As we use specific version like "3.0-SNAPSHOT", we need to repeat modifications between different modules. So why not use a placeholder like ${revision}.

Copy link
Copy Markdown
Member

@whhe whhe left a comment

Choose a reason for hiding this comment

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

If 'revision' property is used, we should also add the flatten-maven-plugin, refer to https://maven.apache.org/maven-ci-friendly.html#install-deploy

@joyCurry30
Copy link
Copy Markdown
Contributor Author

@whhe I add the flatten-maven-plugin. cc plz.

Comment thread pom.xml Outdated
Comment thread pom.xml Outdated
Comment thread pom.xml
Copy link
Copy Markdown
Member

@whhe whhe left a comment

Choose a reason for hiding this comment

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

LGTM, wait for ci to pass.

@whhe whhe merged commit d01f0a9 into apache:master Nov 6, 2023
@leonardBang
Copy link
Copy Markdown
Contributor

Thanks @joyCurry30 and @whhe for the contribution and review, I've one minor question: What's the meaning revision? could we use maven default properties ${project.version} in this case ?

@whhe
Copy link
Copy Markdown
Member

whhe commented Nov 7, 2023

Thanks @joyCurry30 and @whhe for the contribution and review, I've one minor question: What's the meaning revision? could we use maven default properties ${project.version} in this case ?

As we need to declare ${project.version} explicitly in each pom file (the parent part), we need a new property to set a original value for it, and revision is a common option to be used here.

@leonardBang
Copy link
Copy Markdown
Contributor

Thanks @whhe for the explanation, got it.

ChaomingZhangCN pushed a commit to ChaomingZhangCN/flink-cdc that referenced this pull request Jan 13, 2025
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.

4 participants