-
Notifications
You must be signed in to change notification settings - Fork 60
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
docs/howto: negate a disjunction #460
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cue ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
1c4a57b
to
81abb96
Compare
Signed-off-by: Hans van den Bogert <hansbogert@gmail.com>
@jpluscplusm @myitcv I probably lack the permissions to properly ask for a review. Hereby a non-formal review requested. |
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.
This is great - thank you for proposing it! I've requested a few changes, but it feels really close to being mergeable :-)
You'll need to rebase onto the tip of master
and re-run the preprocessor, as we recently merged a change that includes a slightly updated version of CUE that'll invalidate your cache keys.
GitHub doesn't give me a place to review the commit message (unlike the generally nicer Gerrithub.io-based review process!), so I'll do that here:
- Please add a
Preview-Path: /docs/url/path/to/page/
trailer to the commit. - Please include a sentence expanding on the commit message shortlog, explaining what the change includes.
- Please use GitHub's issue-linkng syntax to either
Closes
an issue, or to mark the changeFor
an issue. (If this is onlyFor
an issue, the commit message should outline what remains to be done toCloses
the issue.)
Thanks again - this looks great!
title: Negate a disjunction | ||
tags: | ||
- validation | ||
- language |
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.
We're not yet using the language
tag consistently across the site, so please remove it and we'll re-add it later if it's appropriate for the page.
- language | ||
- commented cue | ||
authors: | ||
- hansbogert |
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.
You'll notice on the preview (https://deploy-preview-460--cue.netlify.app/docs/howto/negate-a-disjunction/) that your author tag doesn't render. To get it to render you'll need to teach the site about yourself: https://cuelang.org/examples/basic/authors/.
- commented cue | ||
authors: | ||
- hansbogert | ||
relatedIssues: |
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.
Thank you for being so thorough by reading existing source docs that you found the existing use of this front matter key! Unfortunately this key isn't actively used in the repo, so doesn't need to be present in the source for this page.
You'll need to track the issues that this page updates and/or closes through the commit message.
--- | ||
|
||
This [Commented CUE]({{< relref "docs/howto#commented-cue-guides" >}}) | ||
demonstrates how to negate a disjunction. |
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 feel this is missing a second sentence that expands slightly on what we mean by "negate a disjunction".
demonstrates how to negate a disjunction. | ||
|
||
{{{with code "en" "example"}}} | ||
! exec cue vet example.cue |
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.
When we have only a single CUE package in scope (in a directory) we omit the CUE file (and package name) that's being evaluated, from the command invocation.
// Define a concrete value | ||
four: "four" | ||
// define a hidden helper value which makes sure | ||
// that `four` is not #oneTwoThree by comparing |
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.
(Another reason to unalign the "four"
value from a field of the same name!)
Let's spell out what "X is not Y" means here, slightly. Something like...
// These hidden fields ensure that the data fields they reference do not contain values permitted by the #oneTwoThree disjunction.
(word-wrapped!)
|
||
{{< info >}} | ||
The method of negating a disjunction shown in this guide is likely to be | ||
replaced by tests using more specific and precise builtins when {{< issue 943 |
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.
The page that you entirely correctly copied this wording from (https://cuelang.org/docs/howto/write-a-type-switch/) uses the concept of "testing" in a slightly different context, in that it's a succession of if
clauses that are literal tests. Here, the anticipated future use of not()
won't be a test, per se, but will be a constraint.
i.e. is likely to be replaced by a constraint using a specific and precise builtin
...
{{< info >}} | ||
The method of negating a disjunction shown in this guide is likely to be | ||
replaced by tests using more specific and precise builtins when {{< issue 943 | ||
/>}} is implemented, such as `not()`. |
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.
Please avoid breaking (source) lines inside shortcodes. It's not wrong, but we're trying to avoid it.
{{< info >}} | ||
The method of negating a disjunction shown in this guide is likely to be | ||
replaced by tests using more specific and precise builtins when {{< issue 943 | ||
/>}} is implemented, such as `not()`. |
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'd end the sentence after "implemented", and not mention the specific builtin name, because the singular "builtin" earlier in the sentence doesn't naturally lead on to a "such as ..." clause.
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 probably lack the native speaker experience here, but isn't "builtins" plural in the source's sentence? But I'll happily leave it out if it confuses.
/edit
Nvm, the singular "builtin" you are referring to is from your proposed change above.
@@ -0,0 +1,53 @@ | |||
--- | |||
title: Negate a disjunction |
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.
You've taken the title from the issue I created (cue-lang/docs-and-content#160), but I'm going to ask @myitcv to confirm that "negate" is the term we want to surface here.
Whatever verb we end up with, please align with these forms across the URL path and page title:
- URL path: /docs/howto/negate-a-disjunction (notice that it reads naturally)
- Page title: Negating a disjunction (this enables it to be formatted on other pages as
Howto Guide: Negating a disjunction
, without requiring the "howto" element to be an integral part of the meaning of the title)
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'll await his answer; complement (set theory) and inverse spring to mind as well.
@jpluscplusm What a pleasant 'first-contact'. Really thorough and elaborated review.
Hint is noted ;) I'll definitely try gerrithub next time. The branch dance with Github isn't really my thing anyway. |
Addresses cue-lang/docs-and-content#160