-
Notifications
You must be signed in to change notification settings - Fork 110
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
add unknown field check to validate command #790
Conversation
Signed-off-by: Mehmet Enes <menes.onus@gmail.com>
✅ Deploy Preview for crossplane ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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 for taking the initiative to make a docs update too @enesonus! a couple of suggestions for you to consider 🙇♂️
Signed-off-by: Mehmet Enes <menes.onus@gmail.com>
@@ -828,7 +828,9 @@ Configuration/platform-ref-aws v0.9.0 True | |||
|
|||
The `crossplane beta validate` command validates | |||
[compositions]({{<ref "../concepts/compositions">}}) against provider or XRD | |||
schemas using the Kubernetes API server's validation library. | |||
schemas using the Kubernetes API server's validation library | |||
with additional validation such as checking for unknown 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.
ha, looks like my suggestion introduced a Vale error in https://github.com/crossplane/docs/actions/runs/9758986765/job/26934740522?pr=790.
content/master/cli/command-reference.md
832:[6](https://github.com/crossplane/docs/actions/runs/9758986765/job/26934740522?pr=790#step:5:7) warning 'additional' is too wordy. write-good.TooWordy
Vale is intended to help us write in a high quality consistent format, but its rules can be difficult to comply with sometimes 🤓
I think this is the rule that is failing:
https://github.com/crossplane/docs/blob/master/utils/vale/styles/write-good/TooWordy.yml#L17
additional
is also mentioned in https://github.com/crossplane/docs/blob/master/utils/vale/styles/Microsoft/ComplexWords.yml#L21, which suggests using more
or extra
instead. So perhaps to make Vale happy we could just use extra
instead of additional
here? 🤔
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.
Yeah I was actually aware of the warning, thought it is Vale's own warning and couldn't find the location to look for alternatives. Thanks for pointing out the guides @jbw976. I changed additional
to extra
. One question is should I leave it as with extra validation
or change it to with extra validations
what do you think? I think the latter one sounds better but we dont have more than one extra validation currently so I couldnt decide 😅
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.
with extra validation
sounds good enough to my ears! 👂 ✅
Signed-off-by: Mehmet Enes <menes.onus@gmail.com>
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.
awesome, this looks great to me now! Thanks @enesonus!!
Adds the improvements made at PR crossplane/crossplane#5791 to
beta validate
command's reference docs for informational purposes.