[Themes] - Refactor pull command to prepare for public themes API
#4500
Conversation
|
Thanks for your contribution! Depending on what you are working on, you may want to request a review from a Shopify team:
|
Coverage report
Test suite run success1894 tests passing in 860 suites. Report generated by 🧪jest coverage report action from c2bceec |
jamesmengo
changed the title
pull command for public themes API
|
We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset. |
| @@ -34,6 +35,12 @@ const COMMANDS = { | |||
| 'theme:share': Share, | |||
| } | |||
|
|
|||
| const PUBLIC_COMMANDS = { | |||
There was a problem hiding this comment.
I've added these to a const here for now, but we're not consuming this in packages/cli/src/index.ts just yet.
Replacing the COMMANDS constant with these new public commands affects the oclif manifests for the CLI (it removes the commands from the base README and oclif.manifest.json.
I will look into this more deeply next week, but any leads are appreciated!
There was a problem hiding this comment.
Thank you, @jamesmengo! The PR is on an excellent direction. I've left only one comment about the documentation in the public API :)
|
|
||
| return | ||
| const pullFlags: PullFlags = { | ||
| path: flags.path, |
There was a problem hiding this comment.
What do you think about adding the documentation in the properties here instead?
Something like this:
/**
* The directory path to download the theme.
*/
path?: stringThe benefit I notice is that users get docs for their properties:
And the documentation of functions gets a bit easier to digest:
There was a problem hiding this comment.
I like this idea!
jamesmengo
force-pushed
the
jm/api
branch
3 times, most recently
from
95b6b11
to
58e4ee5
Compare
|
@karreiro
I think we could consider making some of the arguments required, but I would like to spin my wheels on that and introduce those changes in a followup |
jamesmengo
changed the title
pull command for public themes APIpull command to prepare for public themes API
| @@ -16,7 +21,116 @@ interface PullOptions { | |||
| ignore?: string[] | |||
| } | |||
|
|
|||
| export async function pull(theme: Theme, session: AdminSession, options: PullOptions) { | |||
| export interface PullFlags { | |||
There was a problem hiding this comment.
Update tests Move + fix CLI2 tests Remove pull command test Remove CLI2 Invocation + Tests Remove legacy flag Add JSDoc to pull export Export public pull service from themes package Revert change to theme-command
Re-arrange + export ThemeCommand helpers
WHY are these changes introduced?
Fixes https://github.com/Shopify/develop-advanced-edits/issues/341
Fixes https://github.com/Shopify/develop-advanced-edits/issues/342
Allows users to call
pullprogramatically by providing the flags as argumentsWHAT is this pull request doing?
pullwhich allows users to provide command flags as argspullmethod argsHow to test your changes?
pullinpackages/theme/src/cli/commands/theme/pull.tsto see the Docsshopify-dev theme pullwith and without the--legacyflag. Everything should work as beforeMeasuring impact
How do we know this change was effective? Please choose one:
Checklist