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

Remove PHP template from CLI #4843

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Remove PHP template from CLI #4843

wants to merge 4 commits into from

Conversation

lizkenyon
Copy link
Contributor

WHY are these changes introduced?

  • Remix is the green path for app development at Shopify. The PHP template has been hidden from the CLI for several years, now is the appropriate time to remove it.

WHAT is this pull request doing?

Before
image

After
image

How to test your changes?

  • Attempt to create an app with the following command. Verify that the PHP template is no longer available.
pnpm create-app --template=php
  • Please verify no where else needs to be updated to remove PHP template from the CLI

Post-release steps

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

@lizkenyon lizkenyon requested a review from a team as a code owner November 12, 2024 16:52

This comment has been minimized.

@lizkenyon lizkenyon requested a review from a team as a code owner November 12, 2024 16:55
packages/app/src/cli/models/app/loader.ts Outdated Show resolved Hide resolved
@@ -33,9 +33,8 @@ export default class Dev extends AppCommand {
- Serving [GraphiQL for the Admin API](https://shopify.dev/docs/apps/tools/graphiql-admin-api#use-a-local-graphiql-instance) using your app's credentials and access scopes.
- Building and serving your app and app extensions.
If you're using the PHP or Ruby app template, then you need to complete the following steps before you can preview your app for the first time:
If you're using the Ruby app template, then you need to complete the following steps before you can preview your app for the first time:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this and the bullet point below can be merged together now.

Copy link
Contributor

github-actions bot commented Nov 12, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
71.33% (-2.28% 🔻)
8463/11864
🟡 Branches
67.9% (-1.72% 🔻)
4105/6046
🟡 Functions
70.9% (-2.39% 🔻)
2220/3131
🟡 Lines
71.77% (-2.37% 🔻)
8004/11153
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🔴
... / catalog.ts
0% 100% 0% 0%
🔴
... / generate-file.ts
0% 0% 0% 0%
🔴
... / index.ts
0% 100% 0% 0%
🔴
... / print-ai-prompt.ts
0% 100% 0% 0%
🔴
... / demo.ts
31.46% 0% 4.35% 32.56%
🔴
... / demo-recorder.ts
14.81% 21.43% 27.27% 14.81%
🔴
... / DevServer.ts
5% 0% 0% 5%
🔴
... / InstantaneousMetricReader.ts
0% 0% 0% 0%
🟢
... / types.ts
100% 100% 100% 100%
🔴
... / BaseOtelService.ts
1.89% 0% 0% 1.92%
🔴
... / DefaultMeterProvider.ts
0% 0% 0% 0%
🔴
... / DefaultOtelService.ts
0% 0% 0% 0%
🔴
... / throttle.ts
0% 0% 0% 0%
🔴
... / validators.ts
20% 0% 0% 20%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴
... / json-narrowing.ts
0% (-100% 🔻)
0% (-100% 🔻)
0% (-100% 🔻)
0% (-100% 🔻)
🔴
... / cli.ts
1.39% (-0.02% 🔻)
0% 0%
1.54% (-0.02% 🔻)
🟡
... / error-handler.ts
65% (-1.33% 🔻)
68.52% 57.14%
65.31% (-1.36% 🔻)
🟡
... / output.ts
77.59% (+0.39% 🔼)
75.38% (-0.42% 🔻)
63.64%
77.27% (+0.21% 🔼)
🔴
... / ui.tsx
35.29% (-5.61% 🔻)
36.84% (+1.55% 🔼)
42.42% (+1.8% 🔼)
35.37% (-5.9% 🔻)
🟡
... / prerun.ts
78.13% (-2.52% 🔻)
91.67% 90%
77.42% (-2.58% 🔻)
🔴
... / tunnel.ts
0% (-100% 🔻)
0% (-100% 🔻)
0% (-100% 🔻)
0% (-100% 🔻)

Test suite run success

1917 tests passing in 870 suites.

Report generated by 🧪jest coverage report action from 8554d02

@@ -259,10 +259,9 @@ DESCRIPTION
and access scopes.
- Building and serving your app and app extensions.
If you're using the PHP or Ruby app template, then you need to complete the following steps before you can preview
If you're using the Ruby app template, then you need to complete the following steps before you can preview
Copy link
Contributor Author

Choose a reason for hiding this comment

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

QQ: Is this readme generated from somewhere? Seems to be from dev.ts

Copy link
Contributor

Choose a reason for hiding this comment

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

It is. I think CI checks will nudge you to do this but pnpm refresh-readme should do the job.

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