-
Notifications
You must be signed in to change notification settings - Fork 94
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
Rose Bush utility migration to cylc: incoming #2614
Rose Bush utility migration to cylc: incoming #2614
Conversation
Travis is none too happy, this might be the problem:
|
I will start working through the issues picked up Travis CI etc when I return. Thanks for the heads-up. |
A few name ideas:
Actually, I guess the name of the service does not particularly have to be intuitive or even meaningful, because only |
+21k lines - that's expanded the code base! |
Noticed the CLI |
|
In the, hopefully, not too distant future, we hope to incorporate "cylc nameless" into the "one cylc gui" so it would be good to agree on a naming which is compatible with this...
I like the idea of a name which clarifies the functionality of cylc gui and cylc nameless as this is a source of confusion for new users. |
As regards the favicon png is fine, https://en.wikipedia.org/wiki/Favicon#File_format_support |
Thanks for the suggestions & comments so far all :) I'm working on this on & off (& will comment when the tests adaptation is done) & will read any further comments should you think of anything further to add. In response: The name: Of course it needs to be a joint decision so I think the best way to proceed (if you disagree do say) is for the Metomi team to have a discussion, from which I will summarise the main ideas & points made & report back in a comment here. Hopefully we can narrow our ideas down & try to agree as far as possible as a team before discussing by comment. Though I am glad Oliver commented RE compatibility with future plans as I think that is very important! We will have this discussion sometime next week (as early as possible given people's leave). Favicon, locations... (anything else not mentioned): great, I will leave these be for now. @hjoliver Thanks for alerting me about the |
d67dc0e
to
cb589ee
Compare
@hjoliver, regarding the discussion of the name - various leave meant this was delayed, but we had a conversation about it yesterday, from which I will summarise:
Let us know your thoughts on the above. |
@sadielbartholomew - yes, I like those suggestions. Not entirely sure of "review" but I haven't thought of anything better. |
Thanks @hjoliver, I will bring this up again in our team meeting tomorrow & mention your comment. If we have any more thoughts I will convey them here. There is no particular urgency on the naming decision yet, as I believe we are still waiting on the legal advice & after I fix the tests & any related issues (will get back onto this soon) there is still the review stage which may take a while. |
faaecc1
to
9519dfd
Compare
Re-started work on this PR this afternoon. There are a large number of issues (250+) as highlighted by Codacy, but almost all are from the web-based files & inherited from the original Rose code. Only a small number are in the Python code & I have removed all of the rectifiable Python issues. So (for this PR at least) we should ignore the Codacy failure. I do obviously however need to resolve the failures picked up by Travis CI / the test battery & will do that next. |
603bce0
to
e5abc7d
Compare
There is a minor problem with some of the text in the current rose bush it says:
When it should be:
Could we possibly sneak this one-line change in with this PR 🙏. |
Sure, I'll add that to the checklist. Hope to get through all or most of the items on it today :) |
c800a12
to
11236c6
Compare
624e192
to
5432140
Compare
Thanks @oliver-sanders, good thinking. I've just done that & squashed it into the previous commit. It was a copy & paste apart from reformatting them in line with the Cylc acknowledgement file, adapting the paths to those now in Cylc, & also (note) changing the Bootstrap version link to match that referenced (checking the tweaked one works). Brace yourselves - Codacy is about to spam us again! |
fix broadcast events page
For information, Oliver spotted an issue relating to broadcast events, which he fixed & I have merged in (my bad, this was hard to test as I struggled to find suites with broadcasts). Otherwise, good to go (again)! |
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.
🏅 Approved 🏅
I feel like there should be some ceremony, 🍾 ➡️ ⛵. |
Migration of the 'Rose Bush' HTTP-based service. Closes #1052.
This PR consists of purely the migration to a functional Cylc facility & related 'tidying up' i.e. removing code needed for Rose as a distinct system to retrieve required Cylc data. Follow-up work such as updating the docs & upgrading bootstrap will be completed separately.
Note:
bin/
& two underlib/
, & test files undertests/
, new files are all web-based (HTML, CSS & Javascript) files, under a new directorylib/cylc/cylc-nameless/
with identical structure to that currently inrose/lib/html/
(with files relating torosie disco
removed & only small modifications made otherwise). These are mostly external. Note thatjquery.dataTables.js
constitutes ~3/4 of the ~20,000 line additions for the PR.682b311
,edd18fc
&b15b25b
contain unedited copies of Rose files, to allow relative changes to be seen for aid of review.Checklist:
cylc review
agreed in UM workshop week discussions!.png
scaled crop of the cylc logo indoc/src
, but maybe want something else &/or.ico
format? --> Addressed in PR Made a less fuzzy PNG favicon. sadielbartholomew/cylc-flow#1lib/
directory & forCylcNamelessDAO
class).Open for review & responding to feedback as it is provided.