-
Notifications
You must be signed in to change notification settings - Fork 1
feat(workflows): [CLIENT-3877] adding maven support #88
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
base: main
Are you sure you want to change the base?
Conversation
arrowplum
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.
These changes themselves look good but they don't do enough to make signing work for java I don't think.
Probably something java specific needs to be added to the reusable_sign and the reusable deploy workflows to make sure the actual signed artifacts get picked up.
(from the workflow run in the fluent java) it looks like it is uploading to generic rather than deploying signed artifacts to maven.
86e5fd2 to
66d88b9
Compare
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.
Pull Request Overview
This PR adds Maven/Java artifact support to the shared workflows, enabling building, packaging, and deployment of JAR files alongside existing DEB and RPM package support.
- Adds Java/Maven environment setup configuration to the build workflow
- Implements JAR metadata extraction and Maven repository structuring
- Integrates JAR/POM upload functionality into the deployment pipeline
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
.github/workflows/reusable_execute-build.yaml |
Adds Java setup inputs (version, distribution, cache) and setup-java action step |
.github/workflows/reusable_deploy-artifacts.yaml |
Cosmetic blank line addition for consistency |
.github/workflows/deploy-artifacts/package_utils.sh |
Adds get_jar_metadata() and process_jar() functions for extracting JAR metadata and organizing artifacts into Maven repository structure |
.github/workflows/deploy-artifacts/entrypoint.sh |
Adds JAR artifact structuring, upload logic with deduplication, and exclusion of JAR/POM files from generic processing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I know the tests are horrible. They are failing though with 0 artifacts found so it is something to investigate. I will help. (tests are being rewritten to use an actual test harness) |
Updated comment to reflect returned values Co-authored-by: Copilot <[email protected]>
|
I will address the comments from the copilot. Will spend some time on the tests as well. Thank you @arrowplum |
Updated comment to reflect returned values Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
6f190d7 to
47736d9
Compare
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
Thank you for rebasing in the new testing (using bats). The tests were still broken mostly around test fixtures and nuget upload. This PR (#96 ) addresses these. It is a PR into here rather than into main. |
chore: [CLIENT-3877] add maven tests
|
@arrowplum I have merged your changes. Thank you very much for the help with tests. I think this is ready to go. |
arrowplum
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.
I think we are good 👍
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
arrowplum
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.
Something is wrong with the indention that makes it hard to review. It looks like 8 spaces?
I think the group looks good but please check on this.
| local exit_code=$? | ||
| local line_number=$1 | ||
| echo "Error: Command failed with exit code $exit_code at line $line_number" >&2 | ||
| exit 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.
Something off about the whitespace here. Did trunk fmt not catch it?
This PR adds Maven/Java artifact support to the shared workflows, enabling building, packaging, and deployment of JAR files alongside existing DEB and RPM package support.