-
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
Amend CLI globbing help text. #3302
Conversation
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.
Looks good, apart from one small point, & also see my comment RE the position of the warning from the Cylc-7 twin PR, which also applies here & also in this case to the 'note' for the cycle-point-excluded globbing case (e.g. cylc isnert --help
).
(I'll reposition the warning message on this branch too). |
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.
Good clarifications, I think they will help users to understand the real nature of the commands. There are still a few tweaks need here, ideally.
Oops - @sadielbartholomew I hadn't requested re-review of the this branch yet (just the 7.8.x) - sorry I should have explicitly said I was still working on it. I've already addressed some of your comments (but not pushed yet). However, I had missed the "TASk" typo - good spotting! Sorry again 😬 |
Ah sorry @hjoliver, I guess when you said one was done I assumed the other was too. My apologies! |
b8d1828
to
486096c
Compare
Now ready for review 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.
Tested as displaying the new or amended content with a sensible order & formatting for the --help
output of various affected commands.
Actually hold that for a moment, some of the Travis CI test failures may be legit, so just investigating those. |
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 have doubled checked with Matt who has been dealing with flaky tests, & all Travis CI test failures (see chunk 2/4) are flaky tests which are known to him (one did fail locally for me, but we determinedit was not related to the change here). So they are unrelated, & this is good to go. Thanks.
Update CLI globbing help, post #3296
(We need to be very clear that CLI globbing matches against the current task pool, not abstract tasks in the workflow)
(This may need updating again if we implement pseudo-spawn-on-demand with automatic insertion of tasks not in the pool).
This is a small change with no associated Issue.
Requirements check-list
CONTRIBUTING.md
and added my name as a Code Contributor.