-
Notifications
You must be signed in to change notification settings - Fork 19
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
Xtriggers: Doc Fixes #686
Xtriggers: Doc Fixes #686
Conversation
- Make the extrigger plugins document the eponymous method rather than the module. - Note the existence of validation functions in src/user-guide/writing_workflows/external_triggers. - Give an example of a simple xtrigger validation function.
2cc987c
to
5ecefd3
Compare
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.
After my suggestions to cylc/cylc-flow#5955 are resolved:
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.
* Fix xtrigger entry point example * Update xtrigger plugins docs
- Added "secs" and "subprocess" to dictionary.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Kicking tests |
Ugh, still failing because apparently we run doctests here? |
@@ -146,6 +148,7 @@ ret | |||
retrigger | |||
retriggered | |||
retriggering | |||
rmtree |
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.
Manually adding every argument name to the dictionary is going to get painful.
Any ideas on excluding function signatures and argument lists from spellcheck?
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.
@oliver-sanders I've worked up a proof of concept at https://github.com/cylc/cylc-doc/compare/master...wxtim:cylc-doc:custom_filter_for_Cylc_Words?expand=1 - but I'm not terribly happy with the methods at the top of cylc.doc.filters
which are scraping the Cylc code.
@oliver-sanders Even if I add the required xtrigger imports in a There is apparently no way to exclude So I think we will either have to remove the doctest extension from |
I don't think Have you tried replacing |
Would be good to get this in. Pending:
|
Fine, check that:
If so, we can remove the doctest build from cylc-doc. This build predated our use of pytest which we now configure to run doctests on the source repos. |
Checked; doctests (on master) currently only test PR opened at wxtim#8 |
Xtrigger docs: remove doctest
Co-authored-by: Oliver Sanders <[email protected]>
Recommendation: wxtim#9 |
xtiggers: flesh out validation
…gger It has no effect as `wall_clock` is specially implemented
Hopefully the last recommendation: wxtim#10 |
:members: xrandom, validate | ||
:member-order: bysource |
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.
It is not helpful to provide API reference for this internal interface as it has no valid external application.
:members: xrandom, validate | |
:member-order: bysource | |
:members: xrandom |
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.
True but it does provide a link to the source so it is not useless IMO
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.
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 👍
Companion to cylc/cylc-flow#5955.
Closes #106
Also contains some misc fixes.
make clean
remove src/plugins/xtriggers.xtrigger.xtrigger.__doc__
notxtrigger.__doc__
, which has almost no information.Requirements check-list
CONTRIBUTING.md
and added my name as a Code Contributor.