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.

Sign up for GitHub

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

Jump to bottom

Add sanity check on release notes content #771

Merged
merged 1 commit into from
May 6, 2021
Merged

Add sanity check on release notes content #771

merged 1 commit into from
May 6, 2021

Conversation

qu1queee
Copy link
Contributor

@qu1queee qu1queee commented May 6, 2021 •
edited
Loading

Changes

Fixes #752

Submitter Checklist

  • Includes tests if functionality changed/was added
  • Includes docs if changes are user-facing
  • Set a kind label on this PR
  • 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

Add sanity check on release notes content

@qu1queee qu1queee added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label May 6, 2021
@openshift-ci-robot openshift-ci-robot added the release-note Label for when a PR has specified a release note label May 6, 2021
@qu1queee qu1queee changed the title wip Add sanity check on release notes content May 6, 2021
@openshift-ci-robot openshift-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. and removed release-note Label for when a PR has specified a release note labels May 6, 2021
@qu1queee qu1queee force-pushed the qu1queee/release_notes_validation branch from b6d60bc to af06104 Compare May 6, 2021 13:00
@qu1queee qu1queee added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 6, 2021
@qu1queee qu1queee force-pushed the qu1queee/release_notes_validation branch 7 times, most recently from 35afc5f to 3884f44 Compare May 6, 2021 13:39
@openshift-ci-robot openshift-ci-robot added release-note Label for when a PR has specified a release note and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 6, 2021
@qu1queee qu1queee requested review from HeavyWombat and removed request for xiujuan95 May 6, 2021 13:41
@qu1queee
Adds a sanity check on release notes
00817de 
Validate if the PR contains a release note in the form of:
- NONE
- free text
- action required: free text

if not then fail the check.
@qu1queee qu1queee force-pushed the qu1queee/release_notes_validation branch from 3884f44 to 00817de Compare May 6, 2021 13:50
gabemontero
gabemontero reviewed May 6, 2021
View reviewed changes
.github/workflows/release-notes-linter.yaml
# Validate PR release notes
echo "Going to validate PR ${PR_NUMBER}"

MATCHES=$(wget -q -O- https://api.github.com/repos/shipwright-io/build/pulls/${PR_NUMBER} | jq '.body | match("(```release-note\r\n(.*|NONE|action required: .*)\r\n```)")')
Copy link
Member

Choose a reason for hiding this comment

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

technically speaking not the same arcane bash I derived from tekton and landed with at

build/.github/draft_release_notes.sh

Lines 81 to 82 in b56b30f

PR_BODY=$(wget -q -O- https://api.github.com/repos/shipwright-io/build/issues/${PR_NUM:1})
echo $PR_BODY | grep -oPz '(?s)(?<=```release-note..)(.+?)(?=```)' > /dev/null 2>&1

but to be fair a) I too diverged slightly from tekton when I constructed this to conform to the slight differences we had in out process, and b) you have added the use of jq that I removed, and c) they have tweaked their overall script recently, though git blame says the arcane wget/jq/grep magic has not changed for them in 9 months

for reference https:/tektoncd/plumbing/blob/9c93fcf9b764ef4fcb52f9aa34f7f2551d2fe804/tekton/resources/release/base/github_release.yaml#L133-L135

and there has been talk of switching all this from bash to golang if we get too fed up with things :-)

long winded build up to:

"Unless we encounter discrepancies when cutting release notes next time, and if this passes PR tests, let's go with this , and only iterate further if needed" :-)

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 6, 2021
@gabemontero
Copy link
Member

/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 May 6, 2021
@openshift-merge-robot openshift-merge-robot merged commit 8394c52 into shipwright-io:master May 6, 2021
@qu1queee qu1queee deleted the qu1queee/release_notes_validation branch May 6, 2021 14:22
@adambkaplan adambkaplan added this to the release-v0.5.0 milestone Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gabemontero gabemontero gabemontero left review comments

@imjasonh imjasonh Awaiting requested review from imjasonh

@HeavyWombat HeavyWombat Awaiting requested review from HeavyWombat

Assignees

@gabemontero gabemontero

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 Label for when a PR has specified a release note
Projects
None yet
Milestone
release-v0.5.0
Development

Successfully merging this pull request may close these issues.

Add Sanity Check on Release Notes content
5 participants
@qu1queee @gabemontero @openshift-ci-robot @adambkaplan @openshift-merge-robot