Skip to content
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

cylc8 first-pass #148

Merged
merged 19 commits into from
Aug 24, 2020
Merged

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Aug 17, 2020

Sibling PRs: cylc/cylc-sphinx-extensions#29, cylc/cylc-flow#3777

A quick first-pass of the documentation:

  • Reference configurations properly (:cylc:conf:).
  • Fix some Cylc7/8 changes.
  • Remove some obsolete documentation.
  • Make code examples fix the suite design guide.
  • Auto-document more stuff from Cylc Flow.
  • Simplify some wording.
  • Correct some syntax highlighting.
  • Consistently capitilise Cylc (except the cylc command).
  • Merge "architecture" in with the "introduction" to allow the intro section to grow.

This started out as a quick pass to use the cylc-domain e.g. [runtime] -> :cylc:conf:'[runtime]', however, it is very hard to edit just one thing in the documentation at the moment :/

The Grand Scheme Of Things:

This isn't the "great edit" that the docs need, however, it should make that edit a bit easier when it comes.

These docs aren't polished ready for release, however, they are a bit better than before.

Most of the non-indentation changes are in the user guide, namely writing and running suites.

Pre 8.0.0:

Here's a non-exhaustive list of stuff to do to the docs pre-cylc8:

  • Terminology changes (e.g. suite->flow).
  • Platformise stuff after the [platforms] changes.
  • Review the configuration documentation in cylc-flow.
  • Review all the docs ;)
  • Fix the tutorial to Cylc8 changes.
  • Write a Cylc7->8 transition guide.
  • Fix the screenshots as well as all graph / GUI references.
  • Amend installation instructions as they change + write quickstart.
  • Break apart some of the monolithic sections (e.g. writing / running suites) into more digestible parts.

@oliver-sanders oliver-sanders self-assigned this Aug 17, 2020
@oliver-sanders oliver-sanders added the content Addition or modification of documentation label Aug 17, 2020
@oliver-sanders
Copy link
Member Author

oliver-sanders commented Aug 17, 2020

Tests will fail until the next version of cylc-sphinx-extensions is released and the sibling Cylc Flow PR is merged.

To review, checkout the cylc-flow and cylc-sphinx-extensions branches and pip install them.

src/glossary.rst Outdated
the ``suite.rc`` file.
the :cylc:conf:`suite.rc` file.
Copy link
Member

@MetRonnie MetRonnie Aug 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stuff like this is going to conflict every single time with my suite.rc --> flow.cylc PR #142 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll try to get yours in first then!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oliver-sanders - you're deliberately not ditching the term "suite" in this PR?

Copy link
Member Author

@oliver-sanders oliver-sanders Aug 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now yes. Ronnies change should be merged soon so I'll get the rebase! We will need a separate PRs to eradicate "suite" references in cylc-flow/cylc-doc as this is a big job in of itself.

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good start to the big rewrite 👍 Approved with a few minor suggestions. (And one grump about indentation inside script strings ... but I'll stop flogging that dead horse now ... also for consistency reasons as we've ended with mixed styles at this point).

cylc/doc/etc/tutorial/retries-tutorial/suite.rc Outdated Show resolved Hide resolved
src/glossary.rst Outdated Show resolved Hide resolved
src/glossary.rst Outdated
the ``suite.rc`` file.
the :cylc:conf:`suite.rc` file.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oliver-sanders - you're deliberately not ditching the term "suite" in this PR?

src/glossary.rst Show resolved Hide resolved
`GNU General Public License v3.0
<https://www.gnu.org/licenses/gpl-3.0.en.html>`_.


.. [1] Future plans for EcoConnect include additional deterministic regional
weather forecasts and a statistical ensemble.
.. [2] Note that simply overlapping the single cycle point schedules of the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should delete this whole file. It is way out of date (especially now, post-SoD)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, it's full of NWP context too, happy to remove the content and leave the title or else leave the whole thing pending re-write.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove it.

src/user-guide/remote-job-management.rst Show resolved Hide resolved

- task-to-suite messaging via TCP (using ZMQ protocol)
- task-to-suite messaging via non-interactive SSH to the suite host,
then local TCP
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't we decide we need to restore this capability?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, however, we haven't restored it yet (I think still waiting on a decision over whether we can rely on TCP forwarding or whether we should re-implement the Cylc7 SSH-forwarding logic).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure we decided we had to re-implement the old logic - ping @dpmatthews ? Maybe bang a TODO in here at least, so we don't forget to come back and update this section.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we've decided to use the old method. See cylc/cylc-flow#3327.

src/user-guide/suite-storage-etc.rst Outdated Show resolved Hide resolved
src/user-guide/writing-suites.rst Show resolved Hide resolved
src/user-guide/writing-suites.rst Show resolved Hide resolved
@oliver-sanders
Copy link
Member Author

Obviously, don't merge this branch until the tests pass.

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Aug 19, 2020

Added an extra couple of commits onto the end.

I'm not quite sure what happened here but it looks like the migration of extensions into cylc-sphinx-extensions was never properly completed. I must have been in a rush. There were some static files left behind in src/_static.

This was covering up a much bigger issue to do with the installation of static files. Sadly the approach being used to install the static files by fiddling the static_html_path project configuration no longer works. Sphinx doesn't let you fiddle the project config from extensions any more, which, to be fair, makes sense.

So I've added a commit onto the end of cylc/cylc-sphinx-extensions#29 which manually copies static files across into the build directory. It would appear that this is the only real way to do it, this is what other extensions do.

@MetRonnie
Copy link
Member

Tried to test by checking out all the relevant branches on this, cylc-flow and cylc-sphinx-ext but the conf changes are making it a headache, Can we merge #142 first?

@oliver-sanders
Copy link
Member Author

Rebased and deconflicted, tests should now pass (at long last!).

@oliver-sanders
Copy link
Member Author

... Or not, linkcheck failure still to fix...

@oliver-sanders
Copy link
Member Author

Ok, I think it's nearly there, just one last linkcheck issue which requires yet another PR cylc/cylc-flow#3783

@MetRonnie
Copy link
Member

I've tested this with the right branches checked out and it passes. Don't know if you want to wait for the other branches before merging

@oliver-sanders
Copy link
Member Author

I'll wait for the cylc flow branch to be merged and the actions run to pass.

@oliver-sanders
Copy link
Member Author

Tests pass, took a few PRs to cylc-sphinx-extensions, cylc-doc and cylc-flow, but we are now Sphinx3, [platforms] and *.cylc compliant.

@oliver-sanders
Copy link
Member Author

Merging with two approvals.

@oliver-sanders oliver-sanders merged commit 7ba8037 into cylc:master Aug 24, 2020
@oliver-sanders oliver-sanders deleted the cylc-domain-references branch August 24, 2020 08:34
@MetRonnie MetRonnie linked an issue Aug 27, 2020 that may be closed by this pull request
@MetRonnie MetRonnie mentioned this pull request Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Addition or modification of documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Platforms: update docs
4 participants