-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-18607][build] Give the maven module a human readable name #12907
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
Conversation
|
Note that all Also note that the naming convention I show here is based upon my own preference: what I find easy to read. |
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit bcd598e (Wed Jul 15 12:11:46 UTC 2020) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. DetailsThe Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
|
|
||
| <artifactId>flink-examples-streaming-gcp-pubsub_${scala.binary.version}</artifactId> | ||
| <name>flink-examples-streaming-gcp-pubsub</name> | ||
| <name>Flink : Examples : Dist : Streaming Google PubSub</name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dist -> Helper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, can be. I had doubts about this one. The artifact name is build helper yet the purpose is making it available in the dist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I made it consistent with the module artifact name : "Build Helper".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally would rename this to something like "dist helper" or "dist examples" but that is a separate change if desired.
bcd598e to
b4f6dd1
Compare
|
@nielsbasjes Thanks for the update, I will merge this once tests passed. |
aljoscha
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retroactive approval! And I trust Kurts review.
What is the purpose of the change
Make the build output more human readable for the developers van various situations.
Brief change log
Add or change the
<name>of each maven module (i.e. in every pom.xml)Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
The simplest way to test is by doing a
mvn cleanand verify the list of names at the end looks good.Also looking at the change set verify that the names for each module are correct.
In some exceptional cases I made the name reflect a bit more the purpose like in the case of
flink-examples/flink-examples-build-helperwhere the description indicatesThis is a utility module for building example jars to be used in flink-dist.so I named itFlink : Examples : Dist :Fun fact: I noticed that the ordering of the modules during the build is changed a lot by maven because apparently the ordering of the modules in the pom.xml does not meet the configured dependencies between the modules. So maven reorders the build by itself.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation