Skip to content

x/build/cmd/relnote: document RELNOTE marker rules #37752

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

Closed
dmitshur opened this issue Mar 9, 2020 · 14 comments
Closed

x/build/cmd/relnote: document RELNOTE marker rules #37752

dmitshur opened this issue Mar 9, 2020 · 14 comments
Labels
Builders x/build issues (builders, bots, dashboards) Documentation Issues describing a change to documentation. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Member

dmitshur commented Mar 9, 2020

As far as I know at this time, the RELNOTE marker isn't documented anywhere.

I suspect this causes it to be used less often and not to its full potential.

For example, it's possible to write a string other than yes or y and then the entire string will be included in the generated notes. This means it's possible to write a part of release notes via the RELNOTE marker, not just say "yes". I only learned this today when reading cmd/relnote code, and it's possible more people would take advantage of this if they knew it also.

Perhaps it is already documented somewhere and then this bug is about making me learn that. I've looked in the package documentation and the wiki, with no luck. I've only found https://groups.google.com/d/msg/golang-dev/edmMTXNukjE/V2bh9Vt3CAAJ and #20102 where it was originally announced by @bradfitz, but those aren't good sources for long-term documentation.

Update: The RELNOTE marker is documented at https://golang.org/wiki/CodeReview#cl-directives.

/cc @golang/osp-team

@dmitshur dmitshur added Documentation Issues describing a change to documentation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 9, 2020
@gopherbot gopherbot added this to the Unreleased milestone Mar 9, 2020
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Mar 9, 2020
@bcmills
Copy link
Contributor

bcmills commented Mar 9, 2020

To be honest, I would rather we discourage the use of RELNOTE anyway. The RELNOTE marker makes it easy to add essentially a TODO for someone else to write a release note later, but doesn't actually assign anyone in particular to do it.

I think it's fine to defer writing the release notes — particularly if the CL author isn't comfortable writing English prose for publication — but in that case I would prefer that we add an explicit TODO to the release notes (with more context and a specific person assigned to fill it in), or to file a release-blocker+Documentation issue with an explicit assignee.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Mar 9, 2020

I agree that that is the ideal, but we should not raise writing a release note, even a TODO note, as a blocker for external contributors who may not be familiar with our conventions. For those cases, it's easy for a maintainer to add a RELNOTE tag to the CL.

@bcmills
Copy link
Contributor

bcmills commented Mar 9, 2020

@ianlancetaylor, it seems nearly as easy for a maintainer to add an open issue, which would allow significantly more visibility into the context. (For example, the issue can have a specific assignee and would be more visible in milestone tracking.)

Or, for that matter, it's easy enough for a maintainer to immediately send a followup CL to add the release note. I understand why it can be useful to separate the release-note writing from the contributor's CL, but I don't see the benefit in deferring the release notes as technical debt until the end of the cycle, and that is (empirically) how the RELNOTE tag tends to play out.

@ianlancetaylor
Copy link
Contributor

You are assuming that a core team member is involved with every CL, and that is not the case. I really do not want to raise additional barriers to contribution. I agree that RELNOTE is not ideal, but it's not impossible to work with.

@andybons
Copy link
Member

andybons commented Mar 9, 2020

@ianlancetaylor I’m not sure I agree that it’s an unacceptable burden to place on a contributor. There are circumstances that may warrant an exception (English is not their primary language, for example), but as we would expect someone to add adequate documentation next to the code they’ve written, asking them to update an html file as well doesn’t seem too onerous.

It increases the barrier to contribution, but not by much. I’d rather have the effort spread across all authors and not on a select few who should be focused primarily on getting a reliable release out on time rather than collating and writing notes for functionality they may not be familiar with.

@ianlancetaylor
Copy link
Contributor

Your stance is entirely reasonable. Still, I hold to my position: by all means encourage writing release notes where appropriate, but don't require them.

@andybons
Copy link
Member

andybons commented Mar 9, 2020

I’d say we should strongly encourage writing release notes, taking into account the circumstances, of course. Right now it’s the exception. I’d like to move it more toward the norm, especially for long-time contributors.

@bcmills
Copy link
Contributor

bcmills commented Mar 9, 2020

It also doesn't seem onerous to ask contributors who are +2'ing and merging a CL without a core team member involved to file an issue for someone else to follow up with documentation.

That's only a bit more overhead than making a RELNOTE= comment, and would only affect well-established contributors who presumably have an interest in the health of the project anyway.

@bcmills
Copy link
Contributor

bcmills commented Mar 9, 2020

Actually, could GopherBot open an issue as soon as the CL is merged? That might combine the best of both workflows...

@bcmills
Copy link
Contributor

bcmills commented Mar 9, 2020

(See previously #26419 (comment).)

@dmitshur
Copy link
Member Author

dmitshur commented Mar 9, 2020

Yes, gopherbot can do that, but it'll have to take care not make the mistake of no longer finding an issue it previously created, and entering a loop where it starts creating issues non-stop. While it can be humorous at first, that would not be good.

I suggest filing a separate issue for that, since it's not a completely trivial task and it would be helpful to agree on a high-level approach first. Writing the code to implement it should be easy.

@mvdan
Copy link
Member

mvdan commented May 5, 2020

Here's a drive-by idea. If a commit on master results in a change in api/next.go compared to its parent, it should be marked for inclusion in the release notes.

@dmitshur
Copy link
Member Author

dmitshur commented May 20, 2021

I have now found that it is documented briefly at https://golang.org/wiki/CodeReview#cl-directives (as of 2017).

@dmitshur
Copy link
Member Author

I'll close this since I'm now aware of a place I can link to for documentation of the RELNOTE marker (and expand, if more details are needed), to keep the issue tracker more actionable.

There are good suggestions here, which we're taking into account when thinking about how to improve the process of constructing release notes for each major Go release, but they're out of scope for this particular issue.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 20, 2021
@golang golang locked and limited conversation to collaborators May 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) Documentation Issues describing a change to documentation. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants