Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Use site-specific lists of recipes in the RTW #3856
base: main
Are you sure you want to change the base?
Use site-specific lists of recipes in the RTW #3856
Changes from 21 commits
84c6a66
e803843
de0b3bf
bed1f75
db1a048
0b3971e
6c2f04b
50fe052
9127811
7421e67
b785a76
913854c
f93bee9
1ae2940
cc0cd2f
c3bf56c
f3a73c6
5a876a5
0994bc9
64664c6
0ab631c
73232b0
6e3553c
d489c22
3983a7b
6054822
79ac7af
7d7f19c
2bb79fc
9c65801
6a61034
3445729
c484c89
1713154
477013d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Adding a link to more infomation (the first link) is great! π₯³ I wonder whether this second link should be included somewhere within the first link (if that's where more information about the files is located)? π€
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 second and third links were in the first link AS WELL...! Which is how I prefer it - but I merrily defer to the single source of truth doctrine. Despicable duplicate links removed here: 3445729 π
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.
Would it be possible to remove this comment? It's almost identical to the error message below π
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 just wanted them to REALLY know! π€£
Removed here: 6054822
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 know you spent a lot of time investigating this, and apologies if you already told me the answer to this question, but if the file is included (e.g.
{% include 'site/' ~ SITE ~ '-recipes.cylc' %}
) are theFAST_RECIPES
andMEDIUM_RECIPES
variables not recognised? π€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.
Clarify the request. Also, I found out recently that, when writing documentation: use inclusive language that doesn't always refer to "seeing" an element. Unfortunately this is something I used to do a lot and I keep finding occurrences of it! π± I have made a suggestion below, but feel free to adjust π
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 commited your suggestion but have reverted the
.
to a-
(keeping your text) here: 2bb79fcJinja2 renders
.
as a line break.E.g.
I did experiment with getting a
.
(e.g. using\.
,{% raw %}
from here, using an utf-8 full stop) - but it made me feel like too much of a failure. I could live with the dash...!(I didn't comment it in the file; it didn't feel a big enough issue. Let me know if we should have a comment)
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.
Jinja2 code should be indented from the left margin on its own terms, as recommended in Cylc Documentation: Style Guidelines
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.
Done 79ac7af (but yuck! π)
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.
Are single quotes required, or can double quotes be used? π€
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.
Replaced all singles with doubles: 7d7f19c
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.
Would it be possible for you to get these values from the logs from the latest successful run (using your environment), please? π
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.
Added here: 9c65801 I increased memory for two recipes based on the actuals - per the docs. They've obviously been running fine but I thought, if we're the "example" (at least for now) consistency with the documentation was most important.
And I alphabetised the recipes here: 6a61034
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.
Now you have added checks (thank you!), if this was moved to
FAST_RECIPES
(where it belongs), would the workflow run without errors? π€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.
Unfortunately, I don't think so. If
MEDIUM_RECIPES
is empty (list or string) we still get: