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

Add Release Profile for distribution/proxy-native #21657

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

linghengqian
Copy link
Member

@linghengqian linghengqian commented Oct 20, 2022

For #21347.

Changes proposed in this pull request:


Before committing this PR, I'm sure that I have checked the following options:

  • My code follows the code of conduct of this project.
  • I have self-reviewed the commit code.
  • I have (or in comment I request) added corresponding labels for the pull request.
  • I have passed maven check locally : mvn clean install -B -T2C -DskipTests -Dmaven.javadoc.skip=true -e.
  • I have made corresponding changes to the documentation.
  • I have added corresponding unit tests for my changes.

@@ -67,6 +67,35 @@
</dependencies>

<profiles>
<profile>
<id>release</id>
Copy link
Member Author

@linghengqian linghengqian Oct 20, 2022

Choose a reason for hiding this comment

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

For this PR, if you need a binary package of release distribution/proxy-native, you need to execute ./mvnw -am -pl distribution/proxy-native -B -DskipTests -Dmaven.javadoc.skip=true -Dcheckstyle.skip=true - Dspotless.apply.skip=true -Pnative,release clean package. This requires both GraalVM CE for JDK 17 and the native-image component of GraalVM installed on the device on which this command is executed. In which document should I state this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@strongduanmu I just want to make sure, does such a release note exist? 🤣

Copy link
Member

Choose a reason for hiding this comment

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

@linghengqian I think we can add this requirement in ShardingSphere release document——https://shardingsphere.apache.org/community/cn/involved/release/shardingsphere/#3-%E5%8F%91%E5%B8%83-docker.

Copy link
Member Author

Choose a reason for hiding this comment

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

  • I looked at the Dockerfile and release process of distribution/proxy and found that all the raw materials used to build the Docker Image are extracted from the .tar.gz file generated by the release profile.

  • Is this an unwritten rule, or is it possible to make a Docker Image without making an intermediate .tar.gz? I'm not quite sure if .tar.gz is only needed for distributing binaries.

  • If this provision exists, I will fix the Dockerfile of distribution/proxy-native when opening a new PR.

@linghengqian linghengqian marked this pull request as ready for review October 20, 2022 09:59
@linghengqian
Copy link
Member Author

  • Do I have to copy and paste so much LICENSE content? Can I directly refer to the path where the distribution/proxy's LICENSE is located for packaging? 👀

@terrymanu terrymanu merged commit 37b735e into apache:master Oct 21, 2022
@terrymanu terrymanu added this to the 5.2.2 milestone Oct 21, 2022
@linghengqian linghengqian deleted the graal-release branch October 21, 2022 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants