fix PR template

[pritidesai]

Changes

Delete extra space between three backticks ``` and release-note tag. This is causing all new PRs with a bright red label do-not-merge/release-note-label-needed. The PR author has to go and fix it in the PR description. This fix is not easy to spot for any new contributor.

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

• Has Docs included if any changes are user facing
• Has Tests included if any functionality added or changed
• Follows the commit message standard
• Meets the Tekton contributor standards (including
  functionality, content, code)
• Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
• Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
• Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

/kind misc

[imjasonh]

Thank you!

/lgtm

[imjasonh]

/test pull-tekton-pipeline-build-tests

[abayer]

/hold

This all stems from @vdemeester's change in #4929, and #5105 is also trying to make a similar change to this PR. I think some discussion is needed - I'd like to hold off on merging this or #5105 until we've had a discussion in the Pipeline WG.

[pritidesai]

It's becoming very difficult to review pipeline PRs, most of the new PRs created with various red labels.

I am with you and @vdemeester on mandating PR authors to update release-note but not with such syntax error. I am sure @dibyom on the same page with his PR to fix the same syntax 🤣

It's not just confusing to the new contributors - #5260 (comment) 🙃

We have pipeline WG in two weeks on 08/23, I have added this to the agenda for further discussion.

An alternate could be to not include release note section at all but include instructions on how to add it and if not added, specify consequences. All this does not have to be part of PR template but somewhere if it can be documented and add a reference in the PR template.

[dibyom]

Thanks @pritidesai - yeah my motivation was the same. I find the extra space confusing. Open to any ideas to improve this - removing the space just seemed easiest 😆

An #5105 (comment) could be to not include release note section at all but include instructions on how to add it and if not added, specify consequences. All this does not have to be part of PR template but somewhere if it can be documented and add a reference in the PR template.

This could work - one issue I see is that our release note has to be a block that contains the backticks and then the release-note string [```release-note] - I've found it hard to convey this info via docs and have seen folks just use the notes within the three back ticks without the release-note string leading to more confusing release note failures.

[vdemeester]

An alternate could be to not include release note section at all but include instructions on how to add it and if not added, specify consequences. All this does not have to be part of PR template but somewhere if it can be documented and add a reference in the PR template.

I would vote for that approach then. I would keep the "red" labels (for a good reason), but at least we won't merge PRs that should have release-note with empty release notes.. which them make the release work harder…

Kubernetes uses an empty release note (so not red label) but not commented as well, see https://raw.githubusercontent.com/kubernetes/kubernetes/master/.github/PULL_REQUEST_TEMPLATE.md. The only benefit I can see is that at least we see the empty code block so it should "warn" us that the release-note is missing. Something change between #4929 and now (https://raw.githubusercontent.com/tektoncd/pipeline/main/.github/pull_request_template.md). It was commented before, now it is no more commented, which I agree makes it very confusing.

In all honesty I am fine going both ways:

• k8s ways, which is what that PR proposes
• no release-note code block at all (which I prefer)

This could work - one issue I see is that our release note has to be a block that contains the backticks and then the release-note string [```release-note] - I've found it hard to convey this info via docs and have seen folks just use the notes within the three back ticks without the release-note string leading to more confusing release note failures.

Another way could be to update our script gather release-note to look for a code block below a given title (release note), which would make it not require the release-note code block. But then the release-note prow plugin automation cannot be used 🙃.

In the end it's a battle (and a balance) between authoring, reviewing and releasing 😝.
For example, I'd rather have some "red" label that state that the release-note is not set properly (by not being there), and the comment that tells you why, than anything else. Of course it requires contributors to read those comments, which I feel is a decent ask 👼🏼.

But I can be convinced otherwise as well.

[dibyom]

I don't feel too strongly about either approach. I'm fine with the no release-note code block

[wlynch]

/lgtm

Discussions around whether to keep the release-note section aside, can we submit this? I was about to make the same PR because we ran into this in #5643.

Having the non-obvious space in the current template is causing friction for contributors - it looks like a bug with the release notes check, especially since there's no perceivable difference in the rendered GitHub PR markdown.

[imjasonh]

Having the non-obvious space in the current template is causing friction for contributors - it looks like a bug with the release notes check, especially since there's no perceivable difference in the rendered GitHub PR markdown.

+1, this has proven to be confusing and hostile to new contributors, and is repeatedly a stumbling block even for repeat contributors, myself included.

[vdemeester]

/hold cancel
/approve

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

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:

• OWNERS [vdemeester]

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

[vdemeester]

/test pull-tekton-pipeline-alpha-integration-tests