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

[ feature ] Update theme pull/push --only option to accept multiple patterns #2002

Merged

Conversation

kilgore5
Copy link
Contributor

@kilgore5 kilgore5 commented Feb 1, 2022

WHY are these changes introduced?

After #1892, users can use the --only arg to "include" only certain files when using theme pull/push, but only one option pattern is accepted (see comment #1892 (comment))

This PR updates the --only option to allow it to be used multiple times, similarly to how the --ignore option behaves. E.g.
shopify theme pull -o 'templates/*.json' -o 'config/settings_data.json'

WHAT is this pull request doing?

This PR updates the ShopifyCLI::Theme::IncludeFilter to allow for multiple patterns to function similarly to how ShopifyCLI::Theme::IgnoreFilter already does.

How to test your changes?

empty-ish theme repo

$ ls

pull templates and config files

$ SHOPIFY_CLI_STACKTRACE=1 shopify theme pull -d -n -o 'templates/*.json' -o 'config/*.json'
⭑ You are running a development version of the CLI at:
  ~/shopify-cli-kilgore5/bin/shopify

┏━━ Pulling theme files from Development (5de3a3) (#123163869322) on sections-preview.myshopify.com 
┃                                                                                                           100%
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ (0.79s) ━━

see expected files in config and templates directories

$ ls
config		templates

Post-release steps

Update checklist

  • I've added a CHANGELOG entry for this PR (if the change is public-facing)
  • I've considered possible cross-platform impacts (Mac, Linux, Windows).
  • I've left the version number as is (we'll handle incrementing this when releasing).
  • I've included any post-release steps in the section above.

@ghost ghost added the cla-needed label Feb 1, 2022
@kilgore5 kilgore5 changed the title Feature/theme pull allow multiple only options [ feature ] Update theme pull/push --only option to accept multiple patterns Feb 2, 2022
@ghost ghost removed the cla-needed label Feb 2, 2022
@kilgore5 kilgore5 marked this pull request as ready for review February 2, 2022 02:59
@kilgore5 kilgore5 requested review from a team, gonzaloriestra and molly-yu and removed request for a team February 2, 2022 02:59
Copy link
Contributor

@macournoyer macournoyer left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks a lot for this @kilgore5 🙏

Copy link
Contributor

@karreiro karreiro left a comment

Choose a reason for hiding this comment

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

Thank you, @kilgore5! Great stuff 🚀

@karreiro karreiro merged commit e6e718c into Shopify:main Feb 7, 2022
@karreiro
Copy link
Contributor

karreiro commented Feb 7, 2022

@kilgore5 regarding the post-release steps you've mentioned:

Thanks again for the excellent PR!

@kilgore5
Copy link
Contributor Author

kilgore5 commented Feb 8, 2022

Thank you very much for reviewing @macournoyer @karreiro !

@karreiro re: the post-deploy steps...

Screen Shot 2022-02-07 at 9 54 16 PM

@karreiro
Copy link
Contributor

karreiro commented Feb 8, 2022

Thank you for the PR and the clarification, @kilgore5! 🚀

The docs team is taking care of updating the --only option on https://shopify.dev (for pull and push), so we may expect it there soon :)

@kilgore5 kilgore5 deleted the feature/theme-pull-allow-multiple-only-options branch February 15, 2022 03:29
@AdamDemirel
Copy link

Can confirm that this was also an issue for me if specifying multiple --ignore flags. If you use 3 --ignores and log the ignore_pattnerns here: https://github.com/Shopify/shopify-cli/blob/main/lib/project_types/theme/commands/push.rb#L35 You will see that it's getting the first ignore 3 times.

Example:
wrong

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants