[FLINK-32834] Ease local use of tools/ci/compile.sh #23195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- make it easier to work with relative paths - specifically, useful to have the scripts rely on mvnv by default (follow-up)
- support repeated usages of the scripts
- prevent files from being picked up by git/rat - don't use <root>/target because mvn clean would interfere - tmp dirs under tools/ci would be neat, but we lack a central place to create it
The current quoting scheme wasn't compatible if run_mvn points directly to maven. This both blocks a follow-up where we skip maven-utils entirely in local runs, and makes it easier to copy&paste commands to/from the command-line. It was also not really intuitive. At all. This comes with a slight maintenance cost to maven-utils.sh#run_mvn, but given that we'll likely throw this out in 1.19 anyway (because there are better ways to implement this on later maven versions), we can bear that cost.
Ease direct manual usage by reducing the number of parameters.
- ease usage by reducing number of parameters - avoid CI-exclusive maven-utils - use mvnw by default
Copied from the Flink connector release scripts
- run directly against maven by default - avoid CI-exclusive maven-utils
For example: 'tools/ci/compile.sh -Dfast'
XComp
requested changes
Aug 15, 2023
XComp
reviewed
Aug 15, 2023
XComp
requested changes
Aug 15, 2023
I think that we need to come up with a clear place for developer documentation. I didn't think of the README.md at all personally, and was wondering whether it should be added to the flinkDev/building.md docs, but they are also woefully unclear as to what their audience is (users vs dev), and so decided against it. |
XComp
requested changes
Aug 16, 2023
XComp
requested changes
Aug 17, 2023
XComp
approved these changes
Aug 17, 2023
The maven output parsers rely on certain order of messages which can be broken by multi-threaded builds.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR allows devs to run this script locally, just like that. Lower-level scripts are also easier to use.
The commits are quite granular and all have a commit message explaining the change.