-
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
Cylc Tutorial #38
Cylc Tutorial #38
Conversation
* for consistency with sphinx convention
* sphinx extensions coppied from rose documentation
* hyperlinks ing hyperlinks.rst can be used globally
* plugins provide sphinx with svg manipulation tools and * allow sphinx to handle svg conversion where necessary
* required for plugins used by the cylc tutorial
* these scripts/styles underpin the extensions used in the cylc tutorial
* broken by `cylc --version` output change
* vector in preference to derived bitmap * unscaled bitmap in preference to scaled bitmap
* origin: cylc/cylc-ui#129
* display parameterisations, xtriggers, jinja2 discretely
* dependencies * default styling
Follow-on work #39 |
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 was gonna ask how adding a tutorial could result in a |
Yep! |
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.
Furthermore to the review... also followed the tutorial steps, and the hidden answers are a really good idea. The user is first suggested to try to write a graph expression, receives some tips, and can then compare his response with the correct answer. Then next steps are built on top of the previous ones, showing a nice diff of what changed. Oh, and the syntax highlighting works is neatly done too for the suite.rc and other files displayed to the user. There are good diagrams. And also screen shots/figures. The screenshots of Cylc GUI also highlight which part the text is talking about. I tested some of the solutions with Cylc 8, and all the examples run (minus Cylc GUI, of course). If I could change anything, it would be only the parts that talk about observations, forecast, etc, and instead keep examples that are more neutral (but I believe it was inherited from the existing tutorial). I will probably try to follow this new tutorial a few times during the next months to improve my CylcFu! Thanks! |
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.
Builds cleanly (no warnings or errors) under Sphinx v2.0.1
, but I have some comments regarding the content & (some strong opinions on) the theme.
Theme:
You have changed to the Read the Docs theme, claiming it is "required by Sphinx extensions". Surely this single theme is no the only one we can possibly use which interfaces with our extensions (themes are not much more than styling, surely)? What are the other options? I raise this because I think the RtD theme is horrible:
- it is used by countless, & possibly even the majority of software projects, now, so:
- it looks generic & wearisome;
- anyone checking out Cylc for the first time will get the impression we are uncreative, when by using another theme we could stand out;
- it is very hard to navigate through any docs with a large table of contents, as linked via the sidebar, because the sidebar has no scroll functionality, so sometimes I have to scroll the the top or bottom of (sometimes a very long) page to force the sidebar to be dragged up or down to find a page in the list there that I want to go to.
If there are alternative themes we could use, I would like to argue that we at least try some out & have a vote, instead of going with this by default. Looking through the listing of themes here, a lot of them are just as bad, or worse, but there are some gems there.
If compatible with the extensions, I'd like to suggest the 'press' theme (on PyPI here), which is clean, minimal & used by some projects I have seen (& is based on VuePress so has nice Vue-esque styles which may seem familiar to some) but not so much to not stand out or look distinctive. And that has a scroll bar on the sidebar, so our docs could be navigated through with much greater ease. It has most if not all of the benefits of RtD, notably being responsive to window size (e.g. it's mobile readable).
Code block labels
There are some new labels for certain code blocks starting 'Listing X
' for some number label X
. In some cases these are useful, e.g. to show the name of the file the snippet is meant to be from, e.g:
but in others they are not so useful, at least with the 'Listing X' message which is confusing:
(if anything, I would expect 'Listing N
input' & 'Listing N
' (same X=N
) result for these.)
I suspect you were intending to have all of these text labels without the 'Listing X
' part at the start, as the X
labels just seem to be sequential numbering & don't have any meaning with respect to the docs? Simply cutting off that text from the start of all such labels would be fine, as they would then make sense.
Old Tutorial
The old (e.g. in the live 7.8.2
docs) 'Tutorial' is not in here, nor in the master
branch now. Where has the content from that gone? I went to investigate this for myself but it is taking me a while to trace it, so I'll just ask to save time. Has it just been removed completely since 7.8.x
, or perhaps been split up & moved to other sections?
I'm concerned because I didn't see that as a "tutorial" as such, it contained a lot of reference material, so I worry some important information may have been lost if we cut big chunks, or all, of that section without care to check if that information was recorded elsewhere.
Syntax highlighting (existing issue)
I've spotted some instances of errors raised by the Cylc lexer. They're (at the end of the ... html/suite-config.html?highlight=sequential#inter-cycle-triggers
section in the built docs & so) not in the new tutorial, & they were there before (e.g. I can see them in the live 7.8.2
docs), but I raise it since I distinctly recall noticing (if not the same) similar cases with single datetime references in the Rose docs a while back & I think you fixed them there. So is there a fix for that which may be lost in this conversion across?
Otherwise, we can bump this to a new Issue to fix it separately.
src/index.rst
Outdated
|
||
introduction | ||
installation | ||
terminology | ||
tutorial/index | ||
glossary |
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.
That can also be a feature. I'm OK with documentation that shares the same look and feel, but probably because of Javadocs and Maven sites, that normally use the same theme. That helps me to have to think less when I open the documentation for a project. The press theme looks good too, but I'm OK with either here.
You can use your mouse scroll button (or trackpad with one/two fingers depending on hardware?) to scroll the sidebar. Just need to position the mouse on top of the sidebar, then it will scroll separately from the page content. Or if we can change the style a bit, just need to remove +1 on Code Blocks, we don't seem to need Listing in every image, but not sure if that's easy to do with this change, or easier for a future change. On the old Tutorial, maybe the easiest way to locate it would be using the |
I agree with @sadielbartholomew on this - the "press" theme looks much nicer. |
Also agree on removing not-needed code block labels. |
@kinow is correct - it still exists in the 7.8.x branch of cylc-flow. It was removed from cylc-flow master (and cylc-doc) in anticipation of this PR. However, in retrospect I sort of agree with @sadielbartholomew that it wasn't really a "tutorial". It's really a minimal guide to writing and running very simple suites. Perhaps we should restore it as a "Quick Start" guide or similar? |
Theme
Good, shows it's the right choice! Since the "Classic" theme was buggy is this an issue? If this is really a burning problem I'd suggest subtle re-styling of this theme. There are lots of Sphinx themes but few are well maintained to the constantly changing internals of Sphinx. All Sphinx themes seem to have pretty much identical layout so it's best to adapt the standard. Listings
I'm happy to turn listing numbers off in another PR, numbering figures is pretty unusual in modern tech docs. Old TutorialI have no idea where that went, don't think I deleted it. This tutorial should replace it anyway.
It was really "User Guide". Syntax HighlightingThis may have been fixed in Rose, perhaps the |
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 won't push for a different theme, but I would like it to be known that I think the RtD theme is awful (& with that, I will move on from talking about it 😬 & leave us all in peace!).
That aside, good job. The Cylc Tutorial moves to its natural home! 🏡
Some feedback addressed, with the promise that the rest of the items raised as feedback will be added to, or created as, (a) new Issue(s).
close #18
address #11
if travis passes - fix #32
Transfer the Cylc Tutorial and all of its dependent scripts from the Rose Documentation.
cylc --version
outputTried to avoid styling the docs but there is a little in: 2637357, bb7e104. Can revert if controversial.
Otherwise a fairly straight-forward lift and shift. Most code has already been reviewed in Rose, look at individual commits to see changes.