-
Notifications
You must be signed in to change notification settings - Fork 99
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
Possible enhancement - Add changelog for this repo for automating release tags. ❤️☕️💡 #177
Possible enhancement - Add changelog for this repo for automating release tags. ❤️☕️💡 #177
Conversation
i'm not forgetting this PR. Will find some time to review it! |
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
# Create Release with artifacts | ||
- name: Create Kubelogin Release | ||
if: ${{ steps.check-tag-exists.outputs.exists == 'false'}} |
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.
sometimes release may fail when publishing artifacts. would this prevent us from re-running the release?
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.
☕️❤️🥷 Thank you so much for this, I did thought about it, and released we have 2/3 options:
-
Option 1 - In special case where there are no clear build failure signals and artefacts not published we (dev) could just delete that release tag and restart the workflow.
- In case of build failure since it runs under same job the workflow will break early and release tag will never be created.
-
Option 2 - We could separate out these 2 workflow as following:
Publish
workflow - which is responsible for build and artefact publish, then comes,Release tag
workflow - which creates the release et. al.
-
Option 3 - We can explore this: How to prevent creating multiple releases when using a build matrix? actions/create-release#14 (comment) - so we check the url if artefacts are there, if not don't succeed with release tag.
We still be able to automate both the process, Its dev responsibility to first make sure Publish
runs correctly and artefacts are present and then run Create Release
action.
- In the mean time I will see if there is any artefact signal we can check before Crete release tag et. al.
But, please share your thoughts from above and what do you think :) Thank you so much for this discussion.
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.
Typically it's the last step in publishing artifacts that failed. Code building steps are run before releasing. Option 1 is good for me. Would you mind updating readme.md below please?
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.
☕️ sounds good, I will update and push. Thank you so much for this ❤️🙏
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.
@Tatsinnit can you rebase? I just merged another change
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.
btw, in your test release, I don't see published artifacts? is this order right? should release be created before publishing artifacts?
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.
☕️🥷🙏 Thank you so much, sorry was flying (air travelling) hence no replies, now I am land based. Done and successful release link from my fork below. Oh in my test runs, I think it must be release_id
which is getting used for Publish step
which was not obvious before you asked, I will pass it, also I notices that for Publish to Snap Store
step some secret is used so from my fork I removed that step to show the success build: (With Publish to Snap Store
step as well the build and publish happened but I have shared the runner message below.)
- ✅ 🏛️ https://github.com/Tatsinnit/kubelogin/releases/tag/v0.0.126
- (All Green) ✅-> https://github.com/Tatsinnit/kubelogin/actions/runs/4056327559/jobs/6980679484
- (Red with Snap Store ) - I guess due to some secret in use which is not in my Fork ❌ -> https://github.com/Tatsinnit/kubelogin/actions/runs/4056008295/jobs/6979920553
I will add readme changes for a scenario where any failure after successful build and publish happened. (As a recommendation for engineers to delete unsuccessful release tag and release notes.)
Vs Now working:
9395cc2
to
4175c21
Compare
4175c21
to
ccbdad2
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.
LGTM
Intent of the PR
This PR adds the ChangeLog file which will become one file for all the release versions, it kind of follows the keep-the-changelog-standard notion and just one file for all user/PM et. al. to look at to see releases.
Screenshot below is from my fork to show it is working.
What is it and advantages it will give & long term benefits for full automation.
As this project will expand and many new contributor might come, the key advantages I see are :
changelog.md
PR is separate pull request and key stakeholders can review things before release goes out.Thank you so much for reading this and hope this will add value to this repo. ❤️☕️🙏 I hope this helps in long term full automation.
Screenshots: