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

Make tophatting templates changes possible #5134

Merged
merged 3 commits into from
Dec 22, 2024

Conversation

amcaplan
Copy link
Contributor

WHY are these changes introduced?

We should be able to tophat changes without pushing to prod.

WHAT is this pull request doing?

Makes it possible to pass a filepath and test the changes to templates locally. Run with SHOPIFY_APP_TEMPLATES_JSON_PATH=/path/to/your/file and it should work!

How to test your changes?

  1. Run app generate extension normally, it should still work!
  2. Put this file somewhere on your system.
  3. Make a change (for example, delete half the extensions)
  4. Confirm that SHOPIFY_APP_TEMPLATES_JSON_PATH=/path/to/your/file p shopify app generate extension --path /path/to/your/app displays options that match the file

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Just pass `SHOPIFY_APP_TEMPLATES_JSON_PATH=/path/to/your/file` and it
will draw from there instead of pulling from the live URL.
@amcaplan amcaplan requested a review from a team as a code owner December 19, 2024 19:39
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

@amcaplan amcaplan requested a review from shauns December 19, 2024 19:39
Copy link
Contributor

github-actions bot commented Dec 19, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
75.31% (-0.02% 🔻)
8840/11738
🟡 Branches
70.58% (-0.02% 🔻)
4291/6080
🟡 Functions 75.2% 2314/3077
🟡 Lines
75.85% (-0.01% 🔻)
8357/11018
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / app-event-watcher.ts
93.83% (-1.23% 🔻)
86.49% (-2.7% 🔻)
90.48% 98.61%

Test suite run success

1994 tests passing in 900 suites.

Report generated by 🧪jest coverage report action from a8e7cd9

@amcaplan amcaplan force-pushed the make-tophatting-templates-changes-possible branch from 88f17c6 to 8690d6e Compare December 19, 2024 21:42
@amcaplan amcaplan force-pushed the make-tophatting-templates-changes-possible branch from 8690d6e to 762cd7b Compare December 19, 2024 22:16

const TEMPLATE_JSON_URL = 'https://cdn.shopify.com/static/cli/extensions/templates.json'
const TEMPLATE_PATH_ENV_VARIABLE = 'SHOPIFY_APP_TEMPLATES_JSON_PATH'
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's put this in app/src/cli/constants.ts -- there's a handful of other app-specific environment variables there.

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 debated about this point - it's kind of a little debugging hack, but we do have some debugging hacks there as well!

@amcaplan amcaplan enabled auto-merge December 22, 2024 21:47
@amcaplan amcaplan force-pushed the make-tophatting-templates-changes-possible branch from bf6e4ea to a8e7cd9 Compare December 22, 2024 21:50
@amcaplan amcaplan added this pull request to the merge queue Dec 22, 2024
Merged via the queue into main with commit 7aa290c Dec 22, 2024
27 checks passed
@amcaplan amcaplan deleted the make-tophatting-templates-changes-possible branch December 22, 2024 22:03
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.

2 participants