-
Notifications
You must be signed in to change notification settings - Fork 0
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
AJ-1095: Publish Library and CLI #3
Conversation
5132c23
to
3cd63c8
Compare
3cd63c8
to
b68f9d7
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.
A few comments, please feel free to set up a meeting or send me a slack message if you want me to go over the comments in more detail.
ed949c8
to
1e5194e
Compare
remove git setup for cli
move snapshot as an addition on publishing Add new command to readme
update readme
update comment
spotless
c06f0c9
to
0d7456e
Compare
3252657
to
2d82292
Compare
e2272bc
to
ae5ccc7
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.
Just a few more comments, looks OK overall.
.github/workflows/tag.yml
Outdated
@@ -1,9 +1,16 @@ | |||
name: Tag | |||
on: workflow_dispatch | |||
on: | |||
workflow_dispatch: {} |
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 still called on workflow_dispatch
? If not I think you can remove this line.
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.
tag: | ||
needs: [ build, unit-tests-and-sonarqube, source-clear ] | ||
uses: ./.github/workflows/tag.yml | ||
if: success() && github.ref == 'refs/heads/main' | ||
secrets: inherit | ||
|
||
publish-library: | ||
needs: [ tag ] | ||
uses: ./.github/workflows/publish.yml | ||
if: success() && github.ref == 'refs/heads/main' | ||
secrets: inherit | ||
with: | ||
tag: ${{ needs.tag.outputs.tag }} | ||
|
||
release-cli: | ||
needs: [ tag ] | ||
uses: ./.github/workflows/release-cli.yml | ||
secrets: inherit | ||
if: success() && github.ref == 'refs/heads/main' | ||
with: | ||
tag: ${{ needs.tag.outputs.tag }} |
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 I commented on this before, but I can't find the comment now. I think all three steps can be run as part of a single job. Something like
tag-and-release:
needs: [ unit-tests-and-sonarqube, source-clear ]
if: success() && github.ref == 'refs/heads/main'
runs-on: ubuntu-latest
steps:
- name: Tag
uses: .github/workflows/tag.yml
secrets: inherit
- name: Publish library
uses: .github/workflows/publish.yml
with:
tag: ${{ tag.outputs.tag }}
secrets: inherit
- name: Release CLI
uses: .github/workflows/release-cli.yml
with:
tag: ${{ tag.outputs.tag }}
secrets: inherit
Although the yaml lint is complaining about the secrets: inherit
lines. Not sure why, to me it looks the same as the examples here: https://docs.github.com/en/actions/using-workflows/reusing-workflows#using-inputs-and-secrets-in-a-reusable-workflow
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.
Oh sorry, I probably missed that suggestion!
I'd rather keep them as separate jobs. If nothing else, I like the diagram it produces 😀. I also have it so that the published library and publish cli jobs can run at the same time once the tag job completes (not that it saves that much time- they're both quick jobs).
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed! |
Background
The goal of this PR is enable publishing of the java-pfb-library and java-pfb-cli.
Included Changes
Example:
(Note: I've since deleted the release in the screen shot below to avoid confusion, as it was just a dummy release.)
Note on testing:
I've tested all of the individual actions, but I won't really be able to test them all together until this is merged. There is a small chance for a need for a follow on ticket to fix up the github actions.