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

cylc vip (validate-install-play) #5094

Closed
wants to merge 0 commits into from
Closed

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Aug 24, 2022

#3896 - This PR draft, but currently implements the validate install play working practice. I think that it works in principle for the other working practices.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.

@wxtim wxtim self-assigned this Aug 24, 2022
@wxtim wxtim marked this pull request as draft August 24, 2022 08:00
@wxtim wxtim changed the title Implement.vip cylc vip (validate-install-play) Aug 24, 2022
@wxtim wxtim force-pushed the implement.VIP branch 3 times, most recently from 6ec538a to 5787a3a Compare August 31, 2022 13:06
@oliver-sanders oliver-sanders added this to the cylc-8.1.0 milestone Aug 31, 2022
@hjoliver
Copy link
Member

(Looking forward to this - I am now gagging for cylc vip myself 😁 )

@wxtim wxtim marked this pull request as ready for review September 5, 2022 10:20
@wxtim
Copy link
Member Author

wxtim commented Sep 5, 2022

@hjoliver @datamel - I have undrafted this PR - the functionality is ready for review. I'm going to have a look at whether I can add labels to the help output indicating which commands each option applies to - i.e.

-    --workflow-name -n Some info
+    --workflow-name -n [cylc install] Some info

However, I don't see any reason this couldn't be follow in if you wanted to focus on getting this in.

@wxtim wxtim marked this pull request as draft September 5, 2022 12:51
@wxtim wxtim marked this pull request as ready for review September 6, 2022 09:55
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

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

A few comments on the help docs to register while I get on with more thorough manual testing.
I have tested this with a basic workflow, no options just yet, all is working really well 🎉

cylc/flow/scheduler_cli.py Outdated Show resolved Hide resolved
cylc/flow/scheduler_cli.py Outdated Show resolved Hide resolved
cylc/flow/scheduler_cli.py Outdated Show resolved Hide resolved
cylc/flow/scheduler_cli.py Outdated Show resolved Hide resolved
cylc/flow/scheduler_cli.py Outdated Show resolved Hide resolved
cylc/flow/scheduler_cli.py Outdated Show resolved Hide resolved
cylc/flow/scheduler_cli.py Outdated Show resolved Hide resolved
cylc/flow/scheduler_cli.py Outdated Show resolved Hide resolved
@wxtim wxtim closed this Sep 7, 2022
@hjoliver hjoliver mentioned this pull request Sep 29, 2022
7 tasks
@MetRonnie MetRonnie mentioned this pull request Jan 11, 2023
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants