cluster-api-provider-azure
cluster-api-provider-azure copied to clipboard
Update godoc of CapacityReservationGroupID of AzureMachineSpec
What type of PR is this? /kind cleanup
What this PR does / why we need it:
This PR updates the godoc of CapacityReservationGroupID of AzureMachineSpec. The updated godoc contains the explanation of what the requirements are for the CapacityReservationGroupID.
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #4829
Special notes for your reviewer:
- [ ] cherry-pick candidate
TODOs:
- [ ] squashed commits
- [ ] includes documentation
- [ ] adds unit tests
Release note:
NONE
Hi @arkadeepsen. Thanks for your PR.
I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
Thanks @arkadeepsen!
The azuremachine_types.go file is used in generating the CRDs. So you need to run make generate and commit those changes or this will fail the make verify-gen check.
/ok-to-test
Thanks @arkadeepsen!
The
azuremachine_types.gofile is used in generating the CRDs. So you need to runmake generateand commit those changes or this will fail themake verify-gencheck.
@mboersma my bad. Thanks for pointing that out. I have run make generate and pushed the changes.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 62.02%. Comparing base (
43e72a9) to head (de135dc). Report is 20 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #4830 +/- ##
==========================================
+ Coverage 62.01% 62.02% +0.01%
==========================================
Files 201 201
Lines 16858 16858
==========================================
+ Hits 10455 10457 +2
+ Misses 5620 5618 -2
Partials 783 783
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I see the validation is already being done in ValidateCapacityReservationGroupID. I think the clarity helps but other fields that have similar requirements do not include this information in the godoc. @jackfrancis @mboersma what's your take on this? Was just wondering for the sake of consistency, otherwise lgtm.
Providing a context as to why the change in the godoc is introduced. In OpenShift specific APIs, we try to provide information in the godoc which will be beneficial for the user to create/update a CR. Though, as pointed out, the information is provided when the validation fails, it's good to have the information available beforehand. P.S. I made the change to the godoc specific to CapacityReservationGroupID as I had worked on the PR which adds the field.
@willie-yao it's a good point–we could argue it should be documented similarly for many other fields. I just assumed this change in particular would be (would have been) helpful for the requestor and perhaps others, and wasn't bothered by the inconsistency. But we should hear from other maintainers.
LGTM label has been added.
/assign @willie-yao for approval
@mboersma: GitHub didn't allow me to assign the following users: for, approval.
Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide
In response to this:
/assign @willie-yao for approval
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: willie-yao
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [willie-yao]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment