-
Notifications
You must be signed in to change notification settings - Fork 235
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
doc: 1.127 release note #3446
base: master
Are you sure you want to change the base?
doc: 1.127 release note #3446
Conversation
7aa198c
to
0d334c2
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
0d334c2
to
8ede1d5
Compare
docs/releasenotes/release-1.127.md
Outdated
|
||
## New Beta Resources (Direct Reconciler): | ||
* Manage the reference to a shared dataset that a publisher lists in a data exchange. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think it's easier to just list the fields we added. The information here feels redundant vs the field docs. Ideally we would link them, but that seems less important than being precise about what we are adding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, unless there are entirely new resources? I would still just link to the resource (as you are doing) and not add the text. "Manage the reference" made me think we were adding a ref field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you be more specific about list the fields we add
? This adds an entire new resource (beta), not a single field. Do you want me to list all the fields the resource contains? I added the link to the resource reference page which lists all the fields.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I was confused. I don't think we should add the text description, because it duplicates the docs. Better (but not required) to link to the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, unless there are entirely new resources? I would still just link to the resource (as you are doing) and not add the text. "Manage the reference" made me think we were adding a ref field
So adding the text to briefly explain what this resource is is from a discussion with the tech writer. We think it is a better form to adjust the focus to be more user centric. I can remove it here but may still want to keep it in the google release note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I want to add all new fields for a new resource. That seems too verbose. I like the idea of linking the Google resource docs from the Google release notes. As far as I know we don't have OSS resource docs (Maybe we should) but till we do not sure that we have something we should link from these release notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay. Removed the text and keeps the kcc reference link.
docs/releasenotes/release-1.127.md
Outdated
* `Placeholder` | ||
* `GkeHubFeatureMembership` | ||
|
||
* Added `spec.configmanagement.management` to enable Config Sync Auto Upgrade. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would add to "New Fields" and to "modified reconciler". They are telling the user different things: the new field is saying "there's new functionality available". The "Modified Beta Reconciliation" tells the user that if they see a weird regression, it might be because of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the field should be changed. I would keep the reconciliation warning here. Might be worth being more explicit. Eg Fixed bug where false value was being ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Added the item in both places.
Looks good, I don't know if we want to add the field/type summaries, vs linking to the docs. @cheftako might have a different take though. |
8ede1d5
to
d6d0812
Compare
Change description
Fixes #
Tests you have done
make ready-pr
to ensure this PR is ready for review.