docs: break release notes into categories#11217
Conversation
|
cc @envoyproxy/maintainers |
|
Please add feedback on:
|
|
I'm pretty sure some changes will need to be made to |
This attempts to make it easier when upgrading to understand the changes in the release and how they will affect a given deployment. Related: envoyproxy#11211 Signed-off-by: Greg Greenway <ggreenway@apple.com>
2fb465a to
0687e0a
Compare
alyssawilk
left a comment
There was a problem hiding this comment.
Oh this is a really good idea. I really I like splitting out behavioral changes so they're visible, but often there's behavioral changes caused by a new feature (i.e. internal redirects where the default behavior changes due to enhanced config, and it's mostly about the new feature) and there's large overlap between behavioral changes and bug fixes. We could put dual issues in more than one category, but I feel like that gets to be a pain both for remembering to update in all places when you tweak a release note, but also for folks who want to know what change and have to read a note 3 times over because it spans categories.
Just brainstorming and it may be too much trouble, but I wonder if instead we should have one file with tagging, so we can auto-generate a file which is "all notes for this release" but have links at the top of that which bring you to a page listing only bugfixes or only functional changes or only feature add-ons. Then folks who want to see the changes can just look at the full version without duplicates, and folks who want to know say why Vn behaves differently than vN+1 or just want to know what new features they can leverage can get exactly what they want without having to scan categories for changes that fall more into the feature bucket or the bugfix bucket?
|
Thanks this is great, agreed.
I think this is a great idea, though it sounds like a bunch of work. Should we track this idea in an issue and see how it goes with this split and how much duplication there actually ends up being? |
alyssawilk
left a comment
There was a problem hiding this comment.
Fair enough. Do we want to go with duplicates for now? If so I'll wait on @ggreenway to do first pass and then I can comment through the ones I know go in multiple places.
I'm also not clear on what's an incompatible vs minor behavior change. Can we explain in docs somewhere what that difference is?
Good question. In my mind, it was how likely a change is to cause issues. I don't have a good heuristic other than the committer's and reviewer's better judgement. |
alyssawilk
left a comment
There was a problem hiding this comment.
I'd be mildly inclined to just combine the two, but no objections to leaving as-is as long as folks can tell what goes where :-)
Agreed it would be worth it to have some brief discussion of each section in the contributing guide? |
|
I've been thinking about this for the past couple hours. The distinction I've come up with that I feel is generally useful: Whether a change will show a change in behavior for standard-conforming, non-malicious clients, or not. So for example, changing the behavior when an illegal set of headers is included in a request would be a minor change (or possibly a bug fix; the two probably have a lot of overlap). Changing how envoy responds to an upstream |
I think I'd rather have the descriptions inline with the release-notes, even if they get copied for every version. They are needed just as much by the consumer of release notes as by the producer. |
My goal is to give the reader more context about changes, so they know what to look more carefully at. There may be better ways to accomplish this. |
|
FWIW I totally agree with your intent - it'd be nice if folks could focus on changes that are likely to be impactful, and skim over the rest. I guess in the HTTP space I'm less optimistic about how accurate we can be there. Sadly experience tells me breakages aren't highly correlated with the spec (e.g. breakages when we removed whitespace from concatenated headers, or Envoy adding proper-casing for HTTP/1.1 even though the spec allows for lower-cased keys because lower-casing caused problems with so many endpoints). Again I'm not averse to this split as long a we spell it out (and I'm fine with it being here, or in CONTRIBUTING, or both) which it sounds like you're on board with, so have at it! |
Signed-off-by: Greg Greenway <ggreenway@apple.com>
|
Added an iteration trying to describe what different categories mean. The wording will need refinement; I'm just trying to get ideas out right now. Are these the right categories for changes? |
I agree with this. In my latest attempt, I tried to describe it as how likely something is to cause issues. I agree that appealing to the RFC feels nice, but isn't very useful in practice. |
mattklein123
left a comment
There was a problem hiding this comment.
Overall I really like the direction. I left my suggestions for text/format changes but would be very happy to hear the opinion of others. Thank you!
/wait
|
|
||
| Changes | ||
|
|
||
| Incompatible Behavior Changes (things that expected to cause an incompatibility if they apply; changes required) |
There was a problem hiding this comment.
| Incompatible Behavior Changes (things that expected to cause an incompatibility if they apply; changes required) | |
| Incompatible Behavior Changes (changes that are expected to cause an incompatibility if applicable; deployment changes are likely required) |
There was a problem hiding this comment.
style preference: I would probably put the description in italics under the header, same elsewhere.
|
|
||
| * build: official released binary is now built on Ubuntu 18.04, requires glibc >= 2.27. | ||
|
|
||
| Minor Behavior Changes (things that may cause issues for some users, but should not for most) |
There was a problem hiding this comment.
| Minor Behavior Changes (things that may cause issues for some users, but should not for most) | |
| Minor Behavior Changes (changes that may cause incompatibilities for some users, but should not for most) |
| * router: allow retries of streaming or incomplete requests. This removes stat `rq_retry_skipped_request_not_complete`. | ||
| * router: allow retries by default when upstream responds with :ref:`x-envoy-overloaded <config_http_filters_router_x-envoy-overloaded_set>`. | ||
|
|
||
| Bug fixes (expected to improve things; unlikely to have negative effects) |
There was a problem hiding this comment.
| Bug fixes (expected to improve things; unlikely to have negative effects) | |
| Bug fixes (changes expected to improve the state of the world and are unlikely to have negative effects) |
| * prometheus stats: fix the sort order of output lines to comply with the standard. | ||
| * upstream: fixed a bug where Envoy would panic when receiving a GRPC SERVICE_UNKNOWN status on the health check. | ||
|
|
||
| Removed config or runtime |
There was a problem hiding this comment.
Perhaps link to the docs where we explain deprecations?
alyssawilk
left a comment
There was a problem hiding this comment.
Cool, I like the section clarity you've added!
| ---------------- | ||
|
|
||
| * access loggers: applied existing buffer limits to access logs, as well as :ref:`stats <config_access_log_stats>` for logged / dropped logs. This can be reverted temporarily by setting runtime feature `envoy.reloadable_features.disallow_unbounded_access_logs` to false. | ||
| * http: fixed several bugs with applying correct connection close behavior across the http connection manager, health checker, and connection pool. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_connection_close` to false. |
There was a problem hiding this comment.
So do we want to dup issues like this to the bugfix section since they're both, or document (maybe in CONRIBUTING) that when changes overlap sections we add it to the first (as most important)?
There was a problem hiding this comment.
I think we'll have to iterate on this going forward. My current thought is let's put it into the first/most-important section.
Signed-off-by: Greg Greenway <ggreenway@apple.com>
|
|
||
| Removed Config or Runtime | ||
| ------------------------- | ||
| *Normally occurs at the end of the* :ref:`deprecation period <deprecated>` |
There was a problem hiding this comment.
I couldn't figure out a way to have the :ref: be inside the italics/emphasis. If I put the closing * right after the closing backtick of the :ref: then it doesn't render as a link, but instead as the literal text :ref: ..... I also tried putting ** around the link text inside the link, but that didn't work either. My final attempt was putting a space between the closing backtick and the closing *. That resulted in it not being emphasis rendered, but just having a literal * before and after the text.
Any other ideas are welcome, but for now I'm ok with this minor rendering bug in the docs.
Signed-off-by: Greg Greenway <ggreenway@apple.com>
mattklein123
left a comment
There was a problem hiding this comment.
Nice! I think we can ship and iterate.
|
Before (and after) merging, I double-checked that it properly merged in other upstream changes to current.rst, and it all looked good. |
Signed-off-by: Spencer Lewis <slewis@squareup.com> * master: (33 commits) docs: break release notes into categories (envoyproxy#11217) admin: extract more handlers to separate classes (envoyproxy#11258) Load reporting service documentation (envoyproxy#10962) http: testing 304-with-body behavior (envoyproxy#11261) fixing typos and breaking link issues (envoyproxy#11270) devex: initial commit of devcontainer setup (envoyproxy#11207) security: update policy for fix/disclosure SLOs. (envoyproxy#11243) http: fixing CONNECT to not advertise chunk encoding. (envoyproxy#11245) docs: update upstream network filters description (envoyproxy#11231) deps: update datadog tracer to v1.1.5 (envoyproxy#11253) test: Fix missing instantiation of parameterized tests. (envoyproxy#11247) fix go mirror when no changes (envoyproxy#11249) docs: host_rewrite -> host_rewrite_literal (envoyproxy#11229) wasm: update V8 to v8.3.110.9. (envoyproxy#11233) tls: update BoringSSL to 107c03cf (4103). (envoyproxy#11232) bazelci: always exclude nocoverage tag in coverage config (envoyproxy#11226) ci: save api revision in go-control-plane (envoyproxy#11220) build: fix cares build (envoyproxy#11225) stats: Pre-allocate codec stats for http1 and http2 (envoyproxy#11135) api: manifest based edge default documentation. (envoyproxy#11151) ...
Commit Message:
This attempts to make it easier when upgrading to understand the
changes in the release and how they will affect a given deployment.
Related: #11211
Signed-off-by: Greg Greenway ggreenway@apple.com