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
chore: add a composite action for library generation #3173
chore: add a composite action for library generation #3173
Changes from 51 commits
076221c
e854d12
1148792
c80e706
6705561
d31aa8f
3849f7d
6263393
3588cee
814b059
80396c0
c17ca6f
838dab9
44a4c35
925b7c5
541964d
e3063d7
c9cb986
be7329b
12001b0
1e1dd1e
490ce12
d061b2c
87d771f
87cceb3
98d34cc
d2ec86c
da83076
5e18991
2dc8cb9
e0bec4d
b7eadcc
f58ce70
b16e270
8f02497
904d8ad
8a5d21a
3e912b4
8e0824c
3b3a73b
0a5ec38
efa2952
ada44f7
b09661f
c5b39d2
df5e9c7
71e5613
fe3fb3a
6aaa63b
e1bcea7
0f7cc38
3a8fe92
f71f1f3
dda2767
e2f01e2
bbf05a8
764327c
d8f7c6b
ffb6463
f6c764f
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.
What is
PAT
?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.
Personal access token.
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.
Can we use the full name instead of abbreviations since it's not common? See style guide.
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.
Changed to full 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.
I didn't use reusable workflow because, even though the reusable workflow defined here, the context of the runner is associated with the caller workflow.
When a reusable workflow is triggered by a caller workflow, the
github
context is always associated with the caller workflow. See https://docs.github.com/en/actions/sharing-automations/reusing-workflows#overviewThere 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.
So looks like we are just reusing the scripts not the workflow? If that's the case, maybe we just bake the scripts into the image?
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.
Do we still need to volume m2 folder now that generator is baked in?
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'll try to remove it.
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.
So .m2 is still needed?
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.
If I removed this line, the build failed, please see #3173 (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.
Looks like this is a special case of sdk-platform-java because it depends on snapshot jars
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.
From offline discussion, we are dependent on mvn fmt:format, hence a local .m2 volumn is needed. This will be baked into the docker image soon.