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

x/website: make it safer to make changes to draft release notes #37072

Closed
dmitshur opened this issue Feb 6, 2020 · 8 comments
Closed

x/website: make it safer to make changes to draft release notes #37072

dmitshur opened this issue Feb 6, 2020 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Feb 6, 2020

Right now we rely on https://tip.golang.org (served by the golang.org/x/build/cmd/tip command), the server that automatically deploys and serves the latest x/website and Go repository commits, for sharing draft release notes such as at https://tip.golang.org/doc/go1.14.

Always showing the latest Go+x/website commits means that changes to the draft release notes are previewed very quickly, which is great for seeing the latest state and for spotting problems very quickly. However, release notes use template processing, and if there is an error in the template, an error is displayed. That makes editing the draft release notes a risky process, which can cause times where draft release notes can't be viewed (see #37070 recently).

We should find a way to make the process safer, but hopefully without negatively affecting the convenience of being able to see the latest commits without manual effort.

/cc @golang/osp-team

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 6, 2020
@dmitshur dmitshur added this to the Unreleased milestone Feb 6, 2020
@dmitshur
Copy link
Contributor Author

dmitshur commented Feb 6, 2020

Given that the release notes are largely HTML (with a minor template component) inside the doc directory of the main Go repository, and so people often rightfully do not run trybots before submitting changes. A good solution to this should take that into account.

@dmitshur
Copy link
Contributor Author

dmitshur commented Feb 6, 2020

I've searched through the release notes for many Go releases, and I'm seeing that most of them do not actually use the template functionality for anything, other than to escape {{ and }} itself. So a good solution may be to turn off the template functionality, and enable it only when it is needed.

I see that go1.html uses templates quite a bit:

doc/go1.html:
   62  </p>
   63  
   64: {{code "/doc/progs/go1.go" `/greeting := ..byte/` `/append.*hello/`}}
   65  
   66  <p>
   ..
   71  </p>
   72  
   73: {{code "/doc/progs/go1.go" `/append.*world/`}}
   74  
   75  <p>
   ..
  121  </p>
  122  
  123: {{code "/doc/progs/go1.go" `/type Date struct/` `/STOP/`}}
  124  
  125  <p>
   ..

But none of the other releases (1.1 through 1.14) do.

@dmitshur dmitshur self-assigned this Feb 6, 2020
@gopherbot
Copy link

Change https://golang.org/cl/218058 mentions this issue: doc/go1.14: disable text/template processing in HTML page

@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 Feb 6, 2020
@bcmills
Copy link
Contributor

bcmills commented Feb 6, 2020

people often rightfully do not run trybots before submitting changes

#37047 is closely related, in that it would also (ideally) detect issues in generated HTML during TryBot runs.

@gopherbot
Copy link

Change https://golang.org/cl/218217 mentions this issue: content/static/doc: escape template brackets in code.html

@bcmills bcmills changed the title x/website: make it safer to make changes to draft release notes x/website: make it safer to make changes to templated HTML pages Feb 6, 2020
@bcmills
Copy link
Contributor

bcmills commented Feb 6, 2020

This just bit me on CL 217877, too — and it did have a TryBot +1.

I think we really do need to test the rendered templates in some form. (Reopening.)

@bcmills bcmills reopened this Feb 6, 2020
@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. Builders x/build issues (builders, bots, dashboards) labels Feb 6, 2020
gopherbot pushed a commit to golang/website that referenced this issue Feb 6, 2020
I forgot to escape them in CL 217877, and the TryBots don't currently
catch template errors in HTML docs.

Updates golang/go#37042
Updates golang/go#37072

Change-Id: Ia7b82a9c9b85914f224c04050bfbe3747d5a3acc
Reviewed-on: https://go-review.googlesource.com/c/website/+/218217
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@dmitshur
Copy link
Contributor Author

dmitshur commented Feb 6, 2020

The scope of this issue was about the draft release notes, and that's been fixed. I'll revert the issue to reflect that.

Opened #37090 to track testing improvements for catching problems on pages that still use template mode.

@dmitshur dmitshur closed this as completed Feb 6, 2020
@dmitshur dmitshur removed Builders x/build issues (builders, bots, dashboards) Testing An issue that has been verified to require only test changes, not just a test failure. labels Feb 6, 2020
@dmitshur dmitshur changed the title x/website: make it safer to make changes to templated HTML pages x/website: make it safer to make changes to draft release notes Feb 6, 2020
@gopherbot
Copy link

Change https://golang.org/cl/229079 mentions this issue: content: convert doc/{conduct,security}.html to non-template

gopherbot pushed a commit to golang/website that referenced this issue Apr 20, 2020
There are no template features being used in either of these two files,
so convert them to regular non-template HTML files. Having template be
off has proven to be a safer default, and it uses less CPU.

For golang/go#37070.
For golang/go#37072.
For golang/go#37090.

Change-Id: Ib0de115e58c51a9660d648dbb8793fcaae9a7197
Reviewed-on: https://go-review.googlesource.com/c/website/+/229079
Reviewed-by: Alexander Rakoczy <alex@golang.org>
@golang golang locked and limited conversation to collaborators Apr 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants