-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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: full copy-edit of CronWorkflows page #13292
Conversation
- simplify a bunch, per [k8s style guide](https://kubernetes.io/docs/contribute/style/style-guide/#use-simple-and-direct-language) - use active voice and present tense where possible, per [k8s style guide](https://kubernetes.io/docs/contribute/style/style-guide/#use-present-tense) - avoid Latin phrases, per [k8s style guide](https://kubernetes.io/docs/contribute/style/style-guide/#avoid-latin-phrases) - avoid leveling words, per [k8s style guide](https://kubernetes.io/docs/contribute/style/style-guide/#avoid-words-that-assume-a-specific-level-of-understanding) - prefer 1 sentence per line of markdown, per [style guide](https://argo-workflows.readthedocs.io/en/release-3.5/doc-changes/) - reformat the table of options so that it is readable in markdown - don't center the columns, left-align them - there's no reason to center them and this is inconsistent with the rest of the docs; unnecessarily complicated - note that later rows did not do this either - put the ending `|` one space after the row - the very long amount of whitespace would wrap around every row even in my wide screen monitors, despite that only a few options have long descriptions - add a link to IANA and DST similar to the existing cron link - specify that the same cron library is used by k8s `CronJobs` Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
LGTM We still need to add documentation related to the |
Oh, I forgot about that. Also all my other review comments 😅 |
Will you work on that or want me to? just to avoid duplicating efforts |
If you could that'd be great. I have a bit too much on my plate as-is right now. |
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.
As a copy edit this improves things so LGTM.
If anyone is going to do content additions I'd like to see a "Good practice" that cronworkflows should use workflowTemplateRef to trigger templates rather than containing the workflow.
Agreed; we could have a "Best Practices" section at the bottom of this page (and others) |
- fixed merge conflict within the table; removed `stopStrategy` for `release-3.5` Signed-off-by: Anton Gilgur <agilgur5@gmail.com> (cherry picked from commit 5aac5a8)
- fixed merge conflict within the table; removed `stopStrategy` for `release-3.4` Signed-off-by: Anton Gilgur <agilgur5@gmail.com> (cherry picked from commit 5aac5a8)
Motivation
Made a lot of modifications to this page during reviews of #13280 and #12696, so wanted to make the rest match the current style guides.
Also aligns with all my other full copy-edits of docs pages
Modifications
follow the style guides
reformat the table of options so that it is readable in markdown
|
one space after the rowCronJobs
Verification
make docs
passes