Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Updated 'Customise Kedro CLI' #10

Merged
merged 6 commits into from
Jan 19, 2021
Merged

Updated 'Customise Kedro CLI' #10

merged 6 commits into from
Jan 19, 2021

Conversation

andrii-ivaniuk
Copy link
Contributor

@andrii-ivaniuk andrii-ivaniuk commented Jan 15, 2021 •
edited
Loading

@andrii-ivaniuk andrii-ivaniuk self-assigned this Jan 15, 2021
merelcht
merelcht approved these changes Jan 18, 2021
View reviewed changes
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

This looks really good! 👍 Just out of curiosity, is it also possible to just add a single CLI command without the group or do you have to add a group?

stichbury
stichbury reviewed Jan 19, 2021
View reviewed changes
docs/14_extending-kedro.md Outdated
print(context.pipeline.to_json())

...
cli.add_command(jupyter_group)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the jupyter_group here? Is that obvious to others? I'm not seeing that mentioned elsewhere --- can you remove it to avoid confusion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since users need to add some code to the existing file I tried to show (with ...) that some existing code is omitted and cli.add_command(jupyter_group) is a part of that code after which users need to add their code.

stichbury
stichbury reviewed Jan 19, 2021
View reviewed changes
docs/14_extending-kedro.md Outdated
```

After the modification, running `kedro` in your terminal will show the new command.
After the modification, running `kedro -h` in your terminal will show the new `custom` group.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do they need to restart their terminal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No

stichbury
stichbury approved these changes Jan 19, 2021
View reviewed changes
Copy link
Contributor

@stichbury stichbury left a comment

Choose a reason for hiding this comment

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

Nice! Just a few suggestions to make things clear for the reader.

@andrii-ivaniuk
Copy link
Contributor Author

This looks really good! 👍 Just out of curiosity, is it also possible to just add a single CLI command without the group or do you have to add a group?

It is possible but I'd not encourage users to do so. If a user will add e.g. 10 new top-level commands (it is a highly fictional case) it will be a mess. A custom group will help to hide this mess and the user will not be confused with what is Kedro and custom commands.

@andrii-ivaniuk andrii-ivaniuk merged commit 07c3562 into master Jan 19, 2021
@andrii-ivaniuk andrii-ivaniuk deleted the update-customise-Kedro-CLI branch January 19, 2021 14:36
antonymilne pushed a commit to antonymilne/kedro-training that referenced this pull request Mar 18, 2021
@andrii-ivaniuk
Updated 'Customise Kedro CLI' (kedro-org#10)
660d11a
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stichbury stichbury stichbury approved these changes

@merelcht merelcht merelcht approved these changes

@limdauto limdauto Awaiting requested review from limdauto

@lorenabalan lorenabalan Awaiting requested review from lorenabalan

Assignees

@andrii-ivaniuk andrii-ivaniuk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrii-ivaniuk @stichbury @merelcht