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

Amend CLI globbing help text (7.8.x) #3303

Merged
merged 8 commits into from
Aug 23, 2019

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Aug 21, 2019

7.8.x companion of #3302


This is a small change with no associated Issue.

Requirements 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).
  • Does not need tests (CLI help text only).
  • Appropriate change log entry included.
  • No documentation update required (is documentation).

@hjoliver hjoliver added this to the next-release milestone Aug 21, 2019
@hjoliver hjoliver self-assigned this Aug 21, 2019
Copy link
Collaborator

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

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

Just raising a few minor points that we might want to change. One is that I think it might be better to have the warning before the info about task globbing, as that seems more general & important, & the examples at the end mostly concern globbing usage? I.e. from the current form:

<... task info>
To kill one or more tasks, "cylc kill REG TASK_GLOB ..."; to kill all active
tasks: "cylc kill REG".

TASK_GLOB is a pattern to match tasks, or families, or groups of them, in
the task pool:
* [CYCLE-POINT-GLOB/]TASK-NAME-GLOB[:TASK-STATE]
<... other glob examples>

WARNING: this command matches and operates on task proxy instances in the
scheduler task pool, NOT abstract tasks in the workflow. If a task is not
currently represented in the pool you must use "cylc insert" to add it in.

For example, to match:
* all tasks in a cycle: '20200202T0000Z/*' or '*.20200202T0000Z'
<... other examples>

to:

<... task info>
To kill one or more tasks, "cylc kill REG TASK_GLOB ..."; to kill all active
tasks: "cylc kill REG".

WARNING: this command matches and operates on task proxy instances in the
scheduler task pool, NOT abstract tasks in the workflow. If a task is not
currently represented in the pool you must use "cylc insert" to add it in.

TASK_GLOB is a pattern to match tasks, or families, or groups of them, in
the task pool:
* [CYCLE-POINT-GLOB/]TASK-NAME-GLOB[:TASK-STATE]
<... other glob examples>

For example, to match:
* all tasks in a cycle: '20200202T0000Z/*' or '*.20200202T0000Z'
<... other examples>

lib/cylc/option_parsers.py Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
hjoliver and others added 2 commits August 22, 2019 02:05
Co-Authored-By: Sadie L. Bartholomew <30274190+sadielbartholomew@users.noreply.github.com>
Co-Authored-By: Sadie L. Bartholomew <30274190+sadielbartholomew@users.noreply.github.com>
@hjoliver
Copy link
Member Author

@sadielbartholomew - I agree with changing the order as you suggest; will amend the branch tomorrow.

@hjoliver
Copy link
Member Author

Hopefully good to go now @sadielbartholomew. I ended up trying to fix some additional ambiguities in task-targetting CLI help docs.

@hjoliver
Copy link
Member Author

One genuine test failure, damn it.

@hjoliver
Copy link
Member Author

Fixed.

@hjoliver
Copy link
Member Author

(But the PR has not updated with my last pushed commit ... GitHub reacting slowly 🤔 )

@sadielbartholomew
Copy link
Collaborator

GitHub reacting slowly

There's a major outage according to GH status :( I'll wait upon that final commit before reviewing prematurely again!

@hjoliver
Copy link
Member Author

GitHub booted to updated the PR; tests all passed 🎉

@hjoliver hjoliver mentioned this pull request Aug 23, 2019
6 tasks
Copy link
Collaborator

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

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

Testing as displaying new content in the correct manner for the `--help`` output of a sample of the commands influenced by the changes, without any adverse affects., as per the test battery. Great.

@sadielbartholomew sadielbartholomew merged commit 4e21796 into cylc:7.8.x Aug 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants