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

[Themes] Remove requirement to provide name flag in theme rename command #3644

Merged
merged 2 commits into from
Apr 8, 2024

Conversation

jamesmengo
Copy link
Contributor

WHY are these changes introduced?

  • Improvement to the DX for the rename command

WHAT is this pull request doing?

  • Prompt users for the theme name if not provided in a flag

How to test your changes?

  • pnpm test
Google.Chrome.-.Shopify.-.Calendar.-.Week.of.March.31.2024.mp4

Copy link
Contributor

github-actions bot commented Apr 2, 2024

Thanks for your contribution!

Depending on what you are working on, you may want to request a review from a Shopify team:

  • Themes: @shopify/advanced-edits
  • UI extensions: @shopify/ui-extensions-cli
    • Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
  • Hydrogen: @shopify/hydrogen
  • Other: @shopify/cli-foundations

@jamesmengo jamesmengo marked this pull request as ready for review April 2, 2024 19:26
@jamesmengo jamesmengo requested a review from karreiro April 2, 2024 19:26
@jamesmengo jamesmengo self-assigned this Apr 2, 2024

This comment has been minimized.

@jamesmengo jamesmengo added Type: Enhancement New feature or request Area: @shopify/theme @shopify/theme package issues labels Apr 2, 2024
Copy link
Contributor

github-actions bot commented Apr 2, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
72% (-0.14% 🔻)
6752/9378
🟡 Branches
69.21% (+0.04% 🔼)
3297/4764
🟡 Functions
70.72% (-0.26% 🔻)
1799/2544
🟡 Lines
73.14% (-0.14% 🔻)
6367/8705
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / app.ts
84.4% (-0.55% 🔻)
70.69% (-1.44% 🔻)
86.84%
86.02% (-0.44% 🔻)
🟢
... / loader.ts
93.13% (+0.14% 🔼)
86.81% (-0.02% 🔻)
95.12% (+0.39% 🔼)
94.1% (+0.35% 🔼)
🟡
... / select-app.ts
65.38% (-2.47% 🔻)
50% (-16.67% 🔻)
87.5%
68.18% (-1.38% 🔻)

Test suite run success

1617 tests passing in 754 suites.

Report generated by 🧪jest coverage report action from 3386979

@jamesmengo jamesmengo force-pushed the jmeng/removeRequiredFlag branch 5 times, most recently from ec434a6 to b6dc5d7 Compare April 2, 2024 23:27
@jamesmengo jamesmengo force-pushed the jmeng/removeRequiredFlag branch 3 times, most recently from c685147 to 4742383 Compare April 3, 2024 16:31
Copy link
Contributor

github-actions bot commented Apr 4, 2024

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/themes/utils.d.ts
@@ -3,4 +3,5 @@ export declare const DEVELOPMENT_THEME_ROLE = "development";
 export declare const LIVE_THEME_ROLE = "live";
 export declare const UNPUBLISHED_THEME_ROLE = "unpublished";
 export type Role = typeof DEVELOPMENT_THEME_ROLE | typeof LIVE_THEME_ROLE | typeof UNPUBLISHED_THEME_ROLE;
-export declare function isDevelopmentTheme(theme: Theme): boolean;
\ No newline at end of file
+export declare function isDevelopmentTheme(theme: Theme): boolean;
+export declare function promptThemeName(message: string): Promise<string>;
\ No newline at end of file

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, @jamesmengo! 🚀

@jamesmengo jamesmengo added this pull request to the merge queue Apr 8, 2024
Merged via the queue into main with commit e3f5683 Apr 8, 2024
@jamesmengo jamesmengo deleted the jmeng/removeRequiredFlag branch April 8, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: @shopify/theme @shopify/theme package issues Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants