Skip to content
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

Feature/automate changelog note #278

Merged
merged 24 commits into from
Sep 5, 2023

Conversation

Himani1519
Copy link
Contributor

@Himani1519 Himani1519 commented Aug 9, 2023

VERSION: v2.11.0
CHANGELOG: This action making a CHANGELOG note via special syntax from the GitHub PR commit message, like it could automatically update CHANGELOG.md with the message. First job checks if PR body has changelog note or not if it's not there then it asked them to add it and second job is to check if changelog note has been added in changelog.md file or not.

github-actions bot pushed a commit that referenced this pull request Aug 17, 2023
Copy link
Member

@DivergentEuropeans DivergentEuropeans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great start @Himani1519 but I think there's still a couple edgecases here. In hindsight, automating a changelog is very tricky, so I would have probably argued for a changelog automated "check" instead, and if this check fails, then you fail the PR (and then Git action does the logic there in that PR & sees if it can create its own etc. etc. that gets 👍 'd up by the author) but the check is the most important, not writing/guessing what the changelog is. If the author is making the PR, it's not that much extra work to create a CHANGELOG commit versus writing a little "Changelog: xxxx" thing in the PR description. Authors don't create those because they forget and aren't reminded, not lazy. But either way, anyway now that you've started it, this will be even better! BUT it should do that initial check first

id: set-flag
PR_DESCRIPTION="${{ github.event.pull_request.body }}"

# Check if "changelog:" exists in PR description
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that actually true? Will "changelog:" work? Or is it case sensitive?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is, then next line should be updated

- name: Check if CHANGELOG is Updated and Abort if not updated
if: steps.set-flag.outputs.file-flag != 'true'
if ! grep -Fq "${{ steps.extract-changelog.outputs.changelog }}" CHANGELOG.md; then
LINE_NUMBER=$(awk '/^##/{print NR; exit}' CHANGELOG.md)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So looks like LINE_NUMBER will be set to the line number of the first occurrence of a line starting with ## in CHANGELOG.md
which looks good 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, this will fail when the first changelog to be added is a changelog for a new Zowe version. And I have no idea how you'd automate against that, is there something in your Github action that can tell you what Zowe version it is? This is good for now, but here's what could happen:

  1. Zowe PR author forgets changelog in a PR for new Zowe release
  2. Automation adds changelog
  3. Changelog gets committed into CHANGELOG.md into wrong Zowe release section, PR author doesn't know.

You need some way for the PR author to sign off on this. Is there a GitHub action thing where PR is blocked w/o changelog as to enforce it, but the action itself needs to be check somehow by author? Even as something as, if they are responsible for clicking the button, and after a message pops up somewhere that says "Changelog has been added automatically. I tried my best, but check if it's the correct Zowe version!"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another issue:
Forgive me if I'm wrong, my bash is poor. I don't this checks if a CHANGELOG update already exists? ie a user can have a CHANGELOG update in their commits, but this isn't checked for

This could be done via Git action by checking if a commit in the PR contains the words "CHANGELOG"/"changelog"

if: steps.set-flag.outputs.file-flag != 'true'
if ! grep -Fq "${{ steps.extract-changelog.outputs.changelog }}" CHANGELOG.md; then
LINE_NUMBER=$(awk '/^##/{print NR; exit}' CHANGELOG.md)
sed -i "${LINE_NUMBER} a- ${{ steps.extract-changelog.outputs.changelog }} . (#${{ github.event.pull_request.number }})" CHANGELOG.md
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the extra period here because (personal opinion)

changelog.md file or not. . (#278)
looks worse than
changelog.md file or not (#278) or changelog.md file or not. (#278)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure will update that

@pull-request-size pull-request-size bot added size/L and removed size/M labels Aug 21, 2023
echo "::set-output name=version::$VERSION"
else
echo -e "No changelog and version information found in PR description please add them.\n Expected Format:\n VERSION:vX.XX.X\n CHANGELOG:This is changelog note.\n
To re-run the action, just pushed/commit the changes along with PR description, it will automatically triggered by sync."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To re-run the action, just make a push or commit after updating the PR description or updating the changelog via a manual file changing commit.

CHANGELOG.md Outdated
@@ -3,6 +3,7 @@
All notable changes to the Zlux App Server package will be documented in this file.

## v2.11.0
- This action making a CHANGELOG note via special syntax from the GitHub PR commit message, like it could automatically update CHANGELOG.md with the message. First job checks if PR body has changelog note or not if it's not there then it asked them to add it and second job is to check if changelog note has been added in changelog.md file or not. . (#278)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*. .
isn't right

sagar-1310 and others added 22 commits August 29, 2023 20:50
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
@Himani1519 Himani1519 force-pushed the feature/Automate_changelog_note branch from 51328e5 to 3ac948e Compare August 29, 2023 15:21
Copy link
Member

@1000TurquoisePogs 1000TurquoisePogs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After latest changes, looks good now and you can see my tests in
1000TurquoisePogs#7

it does have a failure mode as seen in
1000TurquoisePogs#6
i'm looking forward to that being addressed in a future pr.

Signed-off-by: Himani1519 <[email protected]>
@Himani1519
Copy link
Contributor Author

This is a great start @Himani1519 but I think there's still a couple edgecases here. In hindsight, automating a changelog is very tricky, so I would have probably argued for a changelog automated "check" instead, and if this check fails, then you fail the PR (and then Git action does the logic there in that PR & sees if it can create its own etc. etc. that gets 👍 'd up by the author) but the check is the most important, not writing/guessing what the changelog is. If the author is making the PR, it's not that much extra work to create a CHANGELOG commit versus writing a little "Changelog: xxxx" thing in the PR description. Authors don't create those because they forget and aren't reminded, not lazy. But either way, anyway now that you've started it, this will be even better! BUT it should do that initial check first

Fixed

@Himani1519 Himani1519 closed this Sep 5, 2023
@Himani1519 Himani1519 reopened this Sep 5, 2023
@Himani1519
Copy link
Contributor Author

This is a great start @Himani1519 but I think there's still a couple edgecases here. In hindsight, automating a changelog is very tricky, so I would have probably argued for a changelog automated "check" instead, and if this check fails, then you fail the PR (and then Git action does the logic there in that PR & sees if it can create its own etc. etc. that gets 👍 'd up by the author) but the check is the most important, not writing/guessing what the changelog is. If the author is making the PR, it's not that much extra work to create a CHANGELOG commit versus writing a little "Changelog: xxxx" thing in the PR description. Authors don't create those because they forget and aren't reminded, not lazy. But either way, anyway now that you've started it, this will be even better! BUT it should do that initial check first

Fixed

Signed-off-by: Himani1519 <[email protected]>
@1000TurquoisePogs 1000TurquoisePogs merged commit 5fdd60c into v2.x/staging Sep 5, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants