-
Notifications
You must be signed in to change notification settings - Fork 283
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
perf(ci): only publish artifacts on git version tags of main #3278
perf(ci): only publish artifacts on git version tags of main #3278
Conversation
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 publishing of artifacts shall happen only upon the creation of a tag. Requesting changes for the same
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.
a3a4972
to
e2746f4
Compare
Hello @petermetz , Review point has been addressed. Re-requested for review. Thanks! |
e2746f4
to
408277a
Compare
Congratulations @raynatopedrajeta for your first PR to Hyperledger Cacti ! |
Thanks a lot @jagpreetsinghsasan ! Looking forward for more learnings with the team! |
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.
@raynatopedrajeta Please resolve the merge conflicts, other than that it should be good to go from my side.
@jagpreetsinghsasan Were your concerns addressed? |
Yes @petermetz , the diff looks fine now |
@jagpreetsinghsasan OK, thank you. @raynatopedrajeta Please rebase and resolve the conflicts and then re-request review from both me and @jagpreetsinghsasan and we'll get this merged ASAP. |
408277a
to
2f99830
Compare
Hello @petermetz and @jagpreetsinghsasan , Thanks! |
LGTM. Thanks for a quick rebase, @raynatopedrajeta |
Jagpreet said the review points have been addressed.
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.
@raynatopedrajeta Oops, one more nit: Please fix the ActionLint check that's been failing:
4e4a4df
to
660aff4
Compare
660aff4
to
e73103d
Compare
Hello @petermetz , I have pushed another fix for the action lint. However I can't run the workflows yet since it needs maintainer's approval. Please let me know if there will be more changes. Thanks! Raynato |
Done @raynatopedrajeta !! |
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.
@raynatopedrajeta LGTM, thank you for the updates!
Primary Changes ---------------- 1. Remove auto publishing of container images in all **publish.yaml upon push and maintain automation runs during pull requests Fixes hyperledger-cacti#2360 Signed-off-by: raynato.c.pedrajeta <[email protected]>
e73103d
to
66e3139
Compare
Commit to be reviewed
perf(ci): only publish artifacts on git version tags of main
Fixes #2360
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.