-
Notifications
You must be signed in to change notification settings - Fork 99
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
Update notes for 3.14.0 GA #3728
Conversation
The PR preview for 4bc4d3a is available at theforeman-foreman-documentation-preview-pr-3728.surge.sh The following output files are affected by this PR: |
@ekohl can you review/tag anyone else you think might be useful to get feedback from :) |
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 started to review theforeman/theforeman.org#2221, but the same notes apply. I should have started the review here.
@Lennonka I'd appreciate if you take a look as well
Lenna is on PTO. From my side, the SecureBoot note looks good. |
Deployment and operation in IPv6-only networks is now fully supported. | ||
|
||
Provisioning over IPv6 is supported on bare metal hosts. | ||
For compute resources, you can define the machine outside Satellite on the compute resource and then you can provision the machine as bare metal in Satellite. |
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.
For compute resources, you can define the machine outside Satellite on the compute resource and then you can provision the machine as bare metal in Satellite. | |
For compute resources, you can define the machine outside Foreman on the compute resource and then you can provision the machine as bare metal in Foreman. |
|
||
=== Secure Boot provisioning | ||
|
||
Support for Secure Boot provisioning on bare-metal, VMware vSphere and Libvirt across multiple operating systems. |
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.
@nofaralfasi is this a good addition? I want to also imply that VMs will be created with Secure Boot enabled.
Support for Secure Boot provisioning on bare-metal, VMware vSphere and Libvirt across multiple operating systems. | |
Provisioning with Secure Boot is now supported on bare-metal, VMware vSphere and Libvirt across multiple operating systems. | |
New virtual machines will be created with the default keys enrolled. |
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 don't quite follow. Are you referring to all new VMs being created with the default keys enrolled (which isn't the case)? Or are you talking specifically about new VMs with Secure Boot enabled? If it's the latter, why is it important to mention that here? Isn't that implicit?
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 it's important to mention that you can also create new VMs with Secure Boot enabled. To me provisioning
doesn't always imply creation of VMs. Perhaps then
Support for Secure Boot provisioning on bare-metal, VMware vSphere and Libvirt across multiple operating systems. | |
Provisioning with Secure Boot is now supported on bare-metal, VMware vSphere and Libvirt across multiple operating systems. | |
New virtual machines with Secure Boot enabled will be created with the default keys enrolled. |
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.
LGTM. Thanks!
|
||
=== Invalidate JWT for global registration | ||
|
||
This will give users the ability to edit user permissions and invalidate JWT tokens for other users, invalidate self's token, and admins can invalidate selfs and other user tokens |
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.
In release notes you are talking to the user, not about the user. When you write "This will give users" you're talking in the future tense, but the change has already landed so it should be written in the present tense.
The exact phrasing can probably be best reviewed by a writer, but how about:
This will give users the ability to edit user permissions and invalidate JWT tokens for other users, invalidate self's token, and admins can invalidate selfs and other user tokens | |
Users can now invalidate their own tokens for global registration. | |
Users with the `edit_users` permission can also invalidate all tokens for all users in a single action. |
This will give users the ability to edit user permissions and invalidate JWT tokens for otherers can now invalidate their own tokens for global registration. | ||
Users with the `edit_users` permission can also invalidate all tokens for all users in a single action.users, invalidate self's token, and admins can invalidate selfs and other user tokens |
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'm not sure what happened to this sentence my suggestion, but this doesn't read well to me.
The first sentence is also just wrong in saying that it gives users the ability to edit user permissions. It doesn't. Users that have the edit_users
permission can now invalidate tokens for other users. In my suggestion I tried to split it up into 2 clear cases: something that's true for all users and something that's true for users with a specific permission.
Also, I've never seen self's
before and it really confuses me. I'm not a native speaker, but if I look at https://www.merriam-webster.com/dictionary/self I see no definition that fits. Shouldn't there be a possessive pronoun in this context?
@ekohl I might have missed your original suggestion. please check again |
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.
👍 once you regenerate the Redmine issue links.
Note #3720 does contain an upgrade warning that is probably best to resolve before announcing.
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 went over all the issues and moved them to the correct categories, please update this again.
41ebbb9
to
4bc4d3a
Compare
Adding Headline Features for 3.14.0 Release notes