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

docs: fixes for release notes for 1.20 #18403

Merged
merged 6 commits into from
Oct 5, 2021
Merged

Conversation

alyssawilk
Copy link
Contributor

cleaning up release notes

Risk Level: n/a
Testing: n/a
Docs Changes: yes
Release Notes: no

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk alyssawilk added this to the 1.20.0 milestone Oct 4, 2021
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@@ -138,7 +138,7 @@ New Features
* rbac: added :ref:`matcher<envoy_v3_api_field_config.rbac.v3.Permission.matcher>` along with extension category ``extension_category_envoy.rbac.matchers`` for custom RBAC permission matchers. Added reference implementation for matchers :ref:`envoy.rbac.matchers.upstream_ip_port <extension_envoy.rbac.matchers.upstream_ip_port>`.
* route config: added :ref:`dynamic_metadata <envoy_v3_api_field_config.route.v3.RouteMatch.dynamic_metadata>` for routing based on dynamic metadata.
* router: added retry options predicate extensions configured via
:ref:` <envoy_v3_api_field_config.route.v3.RetryPolicy.retry_options_predicates>`. These
:ref:`<envoy_v3_api_field_config.route.v3.RetryPolicy.retry_options_predicates>`. These
Copy link
Member

Choose a reason for hiding this comment

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

i think you need to remove the <> around the link if there is no link text - im guessing that is what is causing the docs fail

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@@ -1 +1 @@
1.20.0-dev
1.20.0
Copy link
Member

Choose a reason for hiding this comment

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

i thought these changes happened on the release branch, rather than main - but not sure how its done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

per GOVERNANCE.md it's supposed to be with the release PR
the one sketchy bit is me setting the dates to the "wrong day" but given the list we have I'd be shocked if we cut the release today and I'm being optimistic about aiming for tomorrow :-P

@phlax
Copy link
Member

phlax commented Oct 4, 2021

@phlax
Copy link
Member

phlax commented Oct 4, 2021

in "NEW FEATURES" there is a bad link for envoy_v3_api_field_config.overload.v3.OverloadManager.buffer_factory_config i think its missing a preceding space

@alyssawilk
Copy link
Contributor Author

Hey @envoyproxy/envoy-maintainers 1.20.0 release notes here:
with one known reflink issue @phlax caught above. PTAL and suggest other improvements.

https://storage.googleapis.com/envoy-pr/8e2d393/docs/version_history/current.html

@alyssawilk
Copy link
Contributor Author

yeah I wonder if we should format check rst links better - that's two separate bugs in this release and I swear that's half the updates we do nowadays. either way, fix coming soon!

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@phlax
Copy link
Member

phlax commented Oct 4, 2021

I wonder if we should format check rst links bette

i have a PR that does this, but i havent pushed it because i wasnt entirely happy with any of the rst linters, and didnt decide on a way forward

i will tho (and then hide when everyone moans that they have to wait for docs to be built before the rst formatting issues surface 8/)

@alyssawilk
Copy link
Contributor Author

Hm, we could add checks for the 2 failure modes we've seen to the existing rst checks, so at least we don't need to run full docs build for release notes checks. WDYT?

@phlax
Copy link
Member

phlax commented Oct 4, 2021

i think its probs a good idea - ie very shallow rst lint check in formatting checks, and then more thorough check once the rst has been compiled from protos etc

i think the bigger issue is that its not just this file, and running any linter against the current docs creates a humongous list of issues

@phlax
Copy link
Member

phlax commented Oct 4, 2021

some other thoughts on this are

  • i dont want to re/implement our own linter with a ton of regexes, so im ambivalent about adding too many more checks, esp for only one file
  • some checks simply cant be picked up by linters, as they cant know in advance if the rst is valid - eg links to anchors etc

@davinci26
Copy link
Member

LGTM overall, modulo the broken links.

phlax
phlax previously approved these changes Oct 4, 2021
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm

@alyssawilk alyssawilk merged commit 96701cb into envoyproxy:main Oct 5, 2021
@@ -72,6 +72,7 @@ deadline of 3 weeks.
| 1.17.0 | 2020/12/31 | 2021/01/11 | +11 days | 2022/01/11 |
| 1.18.0 | 2021/03/31 | 2021/04/15 | +15 days | 2022/04/15 |
| 1.19.0 | 2021/06/30 | 2021/07/13 | +13 days | 2022/07/13 |
| 1.20.0 | 2021/09/30 | | | |
| 1.20.0 | 2021/09/30 | 2021/10/05 | +5 days | 2022/10/13 |
| 1.20.1 | 2021/12/30 | | | |
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious to know why this is 1.20.1 and not 1.21.0 ?

Copy link
Member

Choose a reason for hiding this comment

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

good spot @suniltheta - do you want to PR to amend ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is the PR #18461

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants