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

update the github action for release notes to use the release note labels to automate release note generation #613

Merged

Conversation

gabemontero
Copy link
Member

@gabemontero gabemontero commented Feb 22, 2021

Changes

The first stage in automating release notes.

As I indicated in the mailing list and slack, if we force use of the kind label in our PRs, I can make a subsequent PR to this that will sort release notes by kind

This borrows from upstream tekton, but translates from being encapsulated by github actions to tekton tasks, as well as makes a few other simplifications given the "simpler nature" of our project.

/assign @adambkaplan
/assign @qu1queee

Submitter Checklist

  • [n/a ] Includes tests if functionality changed/was added
  • [n/a ] Includes docs if changes are user-facing
  • [y ] Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

NONE

@openshift-ci-robot openshift-ci-robot added the release-note-none Label for when a PR does not need a release note label Feb 22, 2021
@gabemontero gabemontero added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Feb 22, 2021
@gabemontero
Copy link
Member Author

An example of the release notes I get in Changes.md when I run HEAD against the v0.3.0 tag:

#595 by qu1queee: Restructure BuildRun reconcile code into separate classes.
#593 by qu1queee: Move controller/reconcile logic into reconciler pkg
#592 by HeavyWombat: Fixed an issue where the build-controller fails to Reconcile with a panic due to a nil pointer dereference. The issue can arise in case there is a failed TaskRun with a pod that has still running (unfinished) containers.

@gabemontero
Copy link
Member Author

I believe this PR's test runs will benefit from @SaschaSchwarze0 's #614

PR_NUM=$(echo $pr | cut -d';' -f3)
PR_RELEASE_NOTE=$(wget -q -O- https://api.github.com/repos/shipwright-io/build/issues/${PR_NUM:1} | jq .body -r | grep -oPz '(?s)(?<=```release-note..)(.+?)(?=```)' | grep -avP '\W*(Your release note here|action required: your release note here|NONE)\W*')
echo -e "$PR_NUM by $AUTHOR: $PR_RELEASE_NOTE" >> Changes.md
done < last-300-prs-with-release-note.txt
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to a bash script that we keep in-tree - example under /hack?

Copy link
Member Author

Choose a reason for hiding this comment

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

Presumably ... just to be clear, what is motivating your ask here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Doing some research on how to do it, it seems perhaps that putting the shell script in the .github folder or .github/actions folder might be more "approved" or "best practice".

From some github dev forums:

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on externalizing this into a script.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok @adambkaplan @qu1queee I've moved the logic to a script file, following the github actions guidance from https://gh.neting.ccmunity/t/running-a-bash-script/141584

@gabemontero
Copy link
Member Author

/refresh

@gabemontero
Copy link
Member Author

let's see if I have to close/reopen this to refresh the e2e list after #614 merged

@gabemontero
Copy link
Member Author

/retest

@gabemontero
Copy link
Member Author

/skip

@gabemontero
Copy link
Member Author

/close

@openshift-ci-robot
Copy link

@gabemontero: Closed this PR.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@gabemontero
Copy link
Member Author

/reopen

@openshift-ci-robot
Copy link

@gabemontero: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@gabemontero
Copy link
Member Author

yep closing / reopening was needed to clear out the new defunct tests

@gabemontero
Copy link
Member Author

ok all green e2e's ... thanks @SaschaSchwarze0

now circling back to pulling new changes into a separate shell script

@adambkaplan @qu1queee my first thought is to update the new script to deal with PR kind labels in a separate PR, once the use of the prow plugin has merged and there a couple of PRs with both release notes and kind labels to test with

@gabemontero
Copy link
Member Author

ok @adambkaplan @qu1queee e2e's are green and new logic broken out into a script file

let the opinions on how to write bash commence :-)

@gabemontero
Copy link
Member Author

/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gabemontero

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 25, 2021
Copy link
Member

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

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

final nits

@@ -0,0 +1,52 @@
# this script assumes the GITHUB_TOKEN and PREVIOUS_TAG environment variables have been set;
Copy link
Member

Choose a reason for hiding this comment

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

Needs a shebang?

Suggested change
# this script assumes the GITHUB_TOKEN and PREVIOUS_TAG environment variables have been set;
#! /bin/sh
# this script assumes the GITHUB_TOKEN and PREVIOUS_TAG environment variables have been set;

Copy link
Member Author

Choose a reason for hiding this comment

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

updated / squashed / pushed @adambkaplan

git log --pretty=format:"%h - %s - %an" ${{ github.event.inputs.tags }}..HEAD | grep -v "Merge pull" >> Changes.md
echo -e "## Features\n\n## Fixes\n\n## Backwards incompatible changes\n\n## Docs\n\n## Misc\n\n## Thanks" >> Changes.md - name: Draft Release
# might not be necessary but make sure
chmod +x "${GITHUB_WORKSPACE}/.github/draft_release_notes.sh"
Copy link
Member

Choose a reason for hiding this comment

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

You may want to ensure that the checked-in bash script has +x permissions. I believe git preserves those.

Copy link
Member Author

Choose a reason for hiding this comment

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

gmontero ~/go/src/github.com/shipwright-io/build  (release-note-2)$ ls -la .github/draft_release_notes.sh 
-rwxrwxr-x. 1 gmontero gmontero 3127 Feb 24 12:36 .github/draft_release_notes.sh
gmontero ~/go/src/github.com/shipwright-io/build  (release-note-2)$

@gabemontero
Copy link
Member Author

updates pushed @adambkaplan

I'll update the PR on whether tests are green if you are waiting on that for lgtm

@qu1queee
Copy link
Contributor

qu1queee commented Feb 26, 2021

@gabemontero let me check this today, not sure why you approve your own pr, I find this strange.

EDIT: I delegated this to Matthias.

Copy link
Contributor

@HeavyWombat HeavyWombat left a comment

Choose a reason for hiding this comment

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

Thanks for the release notes automation. This is awesome. I am looking forward to it. Pretty sure this is help enormously. I ran shellcheck against the file and highlighted some of the issues. Nothing blocking, I think. The only thing I would like to look into is the seemingly trailing extra text.

.github/draft_release_notes.sh Show resolved Hide resolved
.github/draft_release_notes.sh Outdated Show resolved Hide resolved
.github/draft_release_notes.sh Show resolved Hide resolved
.github/draft_release_notes.sh Show resolved Hide resolved
.github/draft_release_notes.sh Outdated Show resolved Hide resolved
.github/draft_release_notes.sh Show resolved Hide resolved
.github/draft_release_notes.sh Outdated Show resolved Hide resolved
.github/workflows/release.yaml Show resolved Hide resolved
@gabemontero
Copy link
Member Author

pushed an update with the changes @HeavyWombat recommended - thanks !!

explanations in the comment threads for changes that were not pulled in

@qu1queee qu1queee requested review from qu1queee and removed request for qu1queee March 2, 2021 20:17
@qu1queee qu1queee removed their assignment Mar 2, 2021
@qu1queee qu1queee requested review from qu1queee and adambkaplan and removed request for qu1queee March 2, 2021 20:17
@imjasonh
Copy link
Contributor

imjasonh commented Mar 5, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 5, 2021
@openshift-merge-robot openshift-merge-robot merged commit fd67f6a into shipwright-io:master Mar 5, 2021
@gabemontero gabemontero deleted the release-note-2 branch March 5, 2021 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note-none Label for when a PR does not need a release note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants