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

DM-24329: add sphinx documentation of the Click CLI in daf_butler #361

Merged
merged 3 commits into from Sep 25, 2020

Conversation

n8pease
Copy link
Contributor

@n8pease n8pease commented Aug 26, 2020

No description provided.

Copy link
Contributor

@MichelleGower MichelleGower 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 typos. I think the documentation will help someone writing a Butler/pipetask subcommand. Question about generic framework bits being in daf_butler instead of a more generic package.

Not directly related to this ticket, but not sure how to take advantage of this nice option and code reuse if using (YAML) config files instead of command line arguments.

It then describes how other packages can add subcommands to the butler command by way of a plugin system,
in `Butler Command Plugins`_.

Finanly, the ``butler`` command line framework can be used to write other commands that load subcommands. This
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling typo in Finally


Shared Options:

- Should be placed in a package that is as high in the dependcy tree as is reasonable for that option.
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling type for dependency

Shared Arguments
~~~~~~~~~~~~~~~~

Arguments definitions can be shared, similar to options, and also should be used to improve consistency and
Copy link
Contributor

Choose a reason for hiding this comment

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

Argument definitions? (The double plural didn't read quite right).


Shared Arguments:

- Should be placed in a package that is as high in the dependcy tree as is reasonable for that option. By
Copy link
Contributor

Choose a reason for hiding this comment

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

Dependency again.

commands that dynamically load subcommands.

It's easy to create a new kind of command by copying the template below and making a few small changes:

Copy link
Contributor

Choose a reason for hiding this comment

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

If there is click framework code that can be used by commands that don't use a butler (or specifically butler command line options), then shouldn't the code be moved outside daf_butler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I spoke with @timj about this. It would be better in a different package, but right now we want to be removing dependencies from daf_butler (as opposed to adding them), and because having it here is not a gen2 deprecation blocker, we're not going to do it right now.

=========================

The ``butler`` commmand uses a ``click.MultiCommand`` subclass called ``LoaderCLI`` that dynamically loads
subcomands from the local package and from plugin packages. ``LoaderCLI`` can be used to implement other
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo subcommands.

kfindeisen and others added 3 commits September 25, 2020 12:58
The documentation for the three command-line scripts has been moved to
separate pages, as recommended by the developer's guide. This makes
the main module page a bit easier to navigate.
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.

None yet

3 participants