-
Notifications
You must be signed in to change notification settings - Fork 19
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
Userguide overhaul #316
Userguide overhaul #316
Conversation
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.
Nice to cut out some of the lengthier sections of cruft.
Some small issues and test failures to fix.
@@ -1204,7 +1213,7 @@ Glossary | |||
The new flow will now continue to run to the end. | |||
|
|||
It is also possible to have multiple :term:`flows <flow>` running in the | |||
scheduler :term:`schduler` simultaneously. | |||
scheduler :term:`scheduler` simultaneously. |
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.
What's the status of the reflow example above?
Can we now document a basic "reflow" workflow (something like start/pause/play/stop)?
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.
Yes, I'll try to document that today, as well as optional outputs.
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.
Bumped to a separate issue - #335
1777704
to
1013082
Compare
Thanks for the early review @oliver-sanders - some things you noted were due to this still being an in-process Draft, but mostly very useful (and time is of the essence right now!) |
e66a286
to
bd33774
Compare
Have now cleaned up most of the |
351b234
to
cb7a04d
Compare
(And rebased onto master, to pick up changes from my previous PR) |
5019665
to
a9128bd
Compare
"Writing Workflows" section almost done now. Turns out, getting through the whole thing in one day was , erm, over-optimistic. 😭 |
f922e05
to
a7661d2
Compare
Undrafted. Let's get what we can in for the final beta. (I have a little more to do as noted in Element chat, but that could be a follow up). |
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.
👍
job states by coloured job icons. | ||
|
||
|
||
.. TODO include task and job state images |
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.
Note, I think we can use the Cylc UI test framework (Cypress) to auto-generate screenshots of UI components e.g. the task/job state key when we get the time.
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.
Partial review
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.
Ongoing review
The scheduler can only check for lateness once a task has appeared in its | ||
active task window. In Cylc 8 this is usually when the task is actually | ||
ready to run, which severely limits the usefulness of late events as | ||
currently implemented. |
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 scheduler can only check for lateness once a task has appeared in its | |
active task window. In Cylc 8 this is usually when the task is actually | |
ready to run, which severely limits the usefulness of late events as | |
currently implemented. | |
The scheduler can only check for lateness once a task has appeared in its | |
active task window. In Cylc 8 this is usually when the task is actually | |
ready to run, which severely limits the usefulness of late events as | |
currently implemented. |
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.
Sphinx doesn't mind, right? Is 3 spaces a convention we've agreed on?
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.
(applied anyway, as it does look a bit nicer in the source file)
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.
Is 3 spaces a convention we've agreed on
Yes.
When we started the cylc-doc we inherited the convention from rose (http://metomi.github.io/rose/doc/html/developing/restructuredtext.html) e.g. heading order =
, -
, ^
which tried to use the prevailing conventions. These are still the prevailing conventions, more or less.
Sphinx doesn't mind, right.
Depends on the version! Mixed indentation did cause us issues in the past when upgrading Sphinx version.
(I think Sphinx might now be going beyond the ReST spec to make things more user friendly now, not sure.)
ReST is highly sensitive to indentation, this is by design, some things might work but not generate the result you intended.
Safer and easier to stick to the prevailing convention.
...
Long answer to aforementioned quibbles since you raised it...
Directives
Directive form 1:
.. directive:: arg1 arg2
:opt1: value1
:opt2: value2
content
Directive form 2:
.. directive::
arg1
arg2
:opt1: value1
:opt2: value2
content
Admonitions
In this case the content
is actually passed to the directive as an argument.
.. note::
content
I think Sphinx has some logic to handle this for admonitions, however, it will break other directives so safer to be consistent.
Indentation
"Natural indentation" should be used i.e subsequent lines should align with the start of previous lines, so:
* foo
bar (2 space)
1. foo
bar (3 space)
.. directive::
content (3 space)
Blockquotes and .. code
Code and blockquotes are indentation sensitive:
Paragraph::
Code example
Paragraph
Blockquote
Example: Malformed directive:
.. directive::
two space indented content
Should this be:
<directive>
content
</directive>
Or:
<directive />
<blockquote>
content
</blockquote>
???
Example: Mixed indentation (someone came along later and edited or added to a directive):
.. directive::
content (3 spaces)
more content (2 spaces)
Same ambiguity as before but worse.
Note there is a line reset character, I think its |
, I'm not sure if its a ReST or a Sphinx feature though which muddies the water somewhat.
Bullets and definition lists
.. two bullet lists NOT connected
* one
* two
* three
* one
* two
* three
.. two definition lists NOT connected
foo
foo
bar
bar
baz
baz
foo
foo
bar
bar
baz
baz
Arbitrary white-space matters here.
Headings
Heading order is based on definition order so the following two examples are both correct:
H1
===
H2
---
H3
^^^
H1
^^^
H2
---
H3
===
This is confusing as heck so keep to convention!
Note Sphinx likes threes, three space indent, and three character minimum for heading underlines!
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.
@oliver-sanders Is it worth adding that to CONTRIBUTING.md?
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, in a slightly more refined form.
I think this is in the Sphinx docs and in the rose/developing page (I think the rose dev pages are a little out of date).
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.
Ongoing review
.. versionadded:: 8.0.0 | ||
|
||
Breaking graph strings on ``&`` and ``|`` is new at Cylc 8 | ||
Graph strings can be broken on ``&`` and ``|`` as well as ``=>``. |
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 placement of this versionadded
admonition disrupts the example that is being shown imho
cf91a8f
to
bd02738
Compare
src/user-guide/running-workflows/external-command-execution.rst
Outdated
Show resolved
Hide resolved
.. _WorkflowStorageEtc: | ||
|
||
Workflow Storage, Discovery, Revision Control, and Deployment | ||
============================================================= |
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.
Is there not a replacement for this section?
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 think its necessary really. Python (for instance) doesn't bother to document why it doesn't have built in storage and revision control for Python programs - it should be obvious to users. And it seems to me we shouldn't be recommending Rose for the job until some future git-based version, knowing that everyone (even UM Partners) has moved away from svn outside of the MOSRS.
Co-authored-by: Ronnie Dutta <[email protected]>
5f0ee6f
to
f03b291
Compare
f03b291
to
4b564d0
Compare
@MetRonnie - hopefully good to go 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.
🚀
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.
Good pending some minor points (will push changes and get this in!).
@@ -1204,7 +1213,7 @@ Glossary | |||
The new flow will now continue to run to the end. | |||
|
|||
It is also possible to have multiple :term:`flows <flow>` running in the | |||
scheduler :term:`schduler` simultaneously. | |||
scheduler :term:`scheduler` simultaneously. |
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.
Bumped to a separate issue - #335
The scheduler can only check for lateness once a task has appeared in its | ||
active task window. In Cylc 8 this is usually when the task is actually | ||
ready to run, which severely limits the usefulness of late events as | ||
currently implemented. |
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.
Is 3 spaces a convention we've agreed on
Yes.
When we started the cylc-doc we inherited the convention from rose (http://metomi.github.io/rose/doc/html/developing/restructuredtext.html) e.g. heading order =
, -
, ^
which tried to use the prevailing conventions. These are still the prevailing conventions, more or less.
Sphinx doesn't mind, right.
Depends on the version! Mixed indentation did cause us issues in the past when upgrading Sphinx version.
(I think Sphinx might now be going beyond the ReST spec to make things more user friendly now, not sure.)
ReST is highly sensitive to indentation, this is by design, some things might work but not generate the result you intended.
Safer and easier to stick to the prevailing convention.
...
Long answer to aforementioned quibbles since you raised it...
Directives
Directive form 1:
.. directive:: arg1 arg2
:opt1: value1
:opt2: value2
content
Directive form 2:
.. directive::
arg1
arg2
:opt1: value1
:opt2: value2
content
Admonitions
In this case the content
is actually passed to the directive as an argument.
.. note::
content
I think Sphinx has some logic to handle this for admonitions, however, it will break other directives so safer to be consistent.
Indentation
"Natural indentation" should be used i.e subsequent lines should align with the start of previous lines, so:
* foo
bar (2 space)
1. foo
bar (3 space)
.. directive::
content (3 space)
Blockquotes and .. code
Code and blockquotes are indentation sensitive:
Paragraph::
Code example
Paragraph
Blockquote
Example: Malformed directive:
.. directive::
two space indented content
Should this be:
<directive>
content
</directive>
Or:
<directive />
<blockquote>
content
</blockquote>
???
Example: Mixed indentation (someone came along later and edited or added to a directive):
.. directive::
content (3 spaces)
more content (2 spaces)
Same ambiguity as before but worse.
Note there is a line reset character, I think its |
, I'm not sure if its a ReST or a Sphinx feature though which muddies the water somewhat.
Bullets and definition lists
.. two bullet lists NOT connected
* one
* two
* three
* one
* two
* three
.. two definition lists NOT connected
foo
foo
bar
bar
baz
baz
foo
foo
bar
bar
baz
baz
Arbitrary white-space matters here.
Headings
Heading order is based on definition order so the following two examples are both correct:
H1
===
H2
---
H3
^^^
H1
^^^
H2
---
H3
===
This is confusing as heck so keep to convention!
Note Sphinx likes threes, three space indent, and three character minimum for heading underlines!
Co-authored-by: Oliver Sanders <[email protected]>
Co-authored-by: Oliver Sanders <[email protected]>
Co-authored-by: Oliver Sanders <[email protected]>
Co-authored-by: Oliver Sanders <[email protected]>
Test fail is unrelated:
All review suggestions applied. |
Cranking through a pre-8.0 full User Guide check 😫 ...
Companion to #310
Close #248