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.
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
feat: hermetic build scripts to use a single output/generation folder #1987
feat: hermetic build scripts to use a single output/generation folder #1987
Changes from 15 commits
0975267
4fd3973
c3e53fc
01c3343
a07b118
e153b00
5c7e51c
3d19e2e
9e87d8c
8b8f24f
22d3e0e
55c7d8c
05858ff
f8271b7
cc57e23
0b54240
0a8e78c
34b9e64
1eba2ed
5ed472b
496c742
0ebc098
80a449e
87b4e73
7fc632a
aa8a1e4
0504984
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 think output_folder will be created by
generate_library.sh
, does it need to be created here?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.
we need external repositories to be located in there as well, which is handled by
generate_showcase.sh
andgenerate_library_integration_test.sh
. I guess it would make more sense to rely on a function to prevent duplicationThere 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.
Fixed in 0ebc098
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.
The reason I wrap this command into
$()
is becausegenerate_library.sh
will exit if the version is incorrect and use$()
to force back to the current shell and execute command after\\
.I'm not sure whether this is the best approach though, maybe worth an investigation later.
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.
Not sure if this approach is good either but I prepended
bash $script
in 7fc632a to prevent the whole test suite from exiting if that's what you mean byforce back to the current shell
. I was having issues when running the unit tests after the adding the OS detection (or a few commits later) function which happened to be fixed by removing$()
.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.
Is this logic correct? I see that showcase integration tests always exit with 1.
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.
It is correct, I found out why they were failing. It was because there was a
pushd cd
(fixed in 0b54240)Now
mvn verify -P enable-golden-tests
behaves correctly