Remove unified deployments beta flag code #2936

matteodepalo · 2023-10-06T11:50:13Z

WHY are these changes introduced?

We still have code that deals with simplified deployments branching logic. Because everyone is on simplified deployments now we can remove that code.

WHAT is this pull request doing?

I've removed what I could find related to unified deployments. We still need to differentiate between releasing or not releasing when doing a deploy so I've changed the mode to a simple release boolean.
Also, we don't need to fetch functions anymore as part of the fetchAppExtensionRegistrations query.

How to test your changes?

Run deploy and deploy --no-release and everything should work.

github-actions · 2023-10-06T11:50:38Z

Thanks for your contribution!

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

Themes: @shopify/theme-code-tools
UI extensions: @shopify/ui-extensions-cli
- Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
Hydrogen: @shopify/hydrogen
Other: @shopify/cli-foundations

github-actions · 2023-10-31T13:16:15Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	72.86% (-0.43% 🔻)	6108/8383
🟡	Branches	70% (-0.41% 🔻)	2968/4240
🟡	Functions	71.51% (-0.33% 🔻)	1564/2187
🟡	Lines	74.06% (-0.4% 🔻)	5800/7831

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / app.ts	85.56% (-1.11% 🔻)	63.16%	83.87% (-3.23% 🔻)	88.61% (-1.27% 🔻)
🟢	`...` / extension-instance.ts	79.79% (-4.91% 🔻)	72.06% (-3.94% 🔻)	87.5% (-3.13% 🔻)	81.82% (-3.9% 🔻)
🟢	`...` / context.ts	89.71% (-0.43% 🔻)	87.79% (-0.7% 🔻)	87.1% (-0.4% 🔻)	90.1% (-0.44% 🔻)
🟢	`...` / deploy.ts	86.11% (-0.47% 🔻)	92.86% (+0.75% 🔼)	87.5% (+8.55% 🔼)	88.24% (-0.08% 🔻)
🟢	`...` / identifiers-extensions.ts	91.94% (+1.17% 🔼)	82.14% (-1.64% 🔻)	100%	92.59% (+1.36% 🔼)
🟢	`...` / identifiers.ts	87.5% (-6.25% 🔻)	80% (-12.31% 🔻)	100%	85.71% (-7.14% 🔻)
🟢	`...` / prompts.ts	83.33% (-3.48% 🔻)	79.17% (-8.33% 🔻)	76% (-4% 🔻)	85.71% (-1.94% 🔻)
🔴	`...` / draftable-extension.ts	28.57% (-4.76% 🔻)	10% (-6.67% 🔻)	33.33%	29.17% (-5.45% 🔻)

Test suite run success

1430 tests passing in 664 suites.

Report generated by 🧪jest coverage report action from b49cce5

github-actions · 2023-10-31T13:45:48Z

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** 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.

gonzaloriestra

Nice work! 🔥

I've checked that everything keeps working and commented a few things that I don't understand, but they are probably fine.

packages/app/src/cli/api/graphql/all_app_extension_registrations.ts

packages/app/src/cli/services/dev/processes/draftable-extension.ts

packages/app/src/cli/services/deploy.ts

mattnoakes

Looks and works great!! I just have some questions about removing what seemed to be legacy functions code from before they moved onto the app module framework.

Also, do you want to keep set_beta_flag.ts around, or can it be removed too?

packages/app/src/cli/services/context/identifiers-extensions.test.ts

packages/app/src/cli/services/context/identifiers.test.ts

packages/app/src/cli/services/context/identifiers.ts

mattnoakes · 2023-10-31T21:51:43Z

packages/app/src/cli/services/context/identifiers-extensions.ts

-  }
+
+  const functionExtensions = options.app.allExtensions.filter((ext) => ext.isFunctionExtension)
+  functionExtensions.forEach((ext) => (ext.usingExtensionsFramework = true))


Can this logic be removed now that all functions are on the app module framework?

packages/app/src/cli/services/context/identifiers-functions.ts

packages/app/src/cli/services/context/prompts.test.ts

matteodepalo · 2023-11-01T09:04:47Z

All good points, thanks @mattnoakes , I'll address them!

mattnoakes

Thanks for addressing our feedback!! Everything still works!

packages/app/src/cli/services/extensions/common.ts

packages/app/src/cli/services/context/identifiers.test.ts

packages/app/src/cli/models/extensions/extension-instance.ts

tests

matteodepalo · 2023-11-02T10:46:36Z

I should have addressed all feedback but for some reason the test coverage run fails 🤔

This reverts commit 59c81b6.

matteodepalo · 2023-11-02T15:19:59Z

I've removed the code that deals with legacy functions, I'd rather have that in a separate PR. @mattnoakes could I get another round of QA just to be extra sure? Everything should still work. If it does we can merge tomorrow.

mattnoakes

Looks good! Deploy works as expected!

mattnoakes · 2023-11-02T15:37:16Z

packages/app/src/cli/services/context/identifiers.test.ts

-  title: 'C',
-  type: 'PRODUCT_DISCOUNTS',
-}
-
 const LOCAL_APP = (uiExtensions: ExtensionInstance[], functionExtensions: ExtensionInstance[] = []): AppInterface => {


Suggested change

const LOCAL_APP = (uiExtensions: ExtensionInstance[], functionExtensions: ExtensionInstance[] = []): AppInterface => {

const LOCAL_APP = (uiExtensions: ExtensionInstance[]): AppInterface => {

I'm going to keep the function related changes separate from this PR, we can go around and clean them up later.

github-actions · 2023-11-06T15:01:05Z

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/ruby.d.ts

@@ -12,7 +12,6 @@ interface ExecCLI2Options {
     signal?: AbortSignal;
     stdout?: Writable;
     stderr?: Writable;
-    unifiedDeployment?: boolean;
 }
 /**
  * Execute CLI 2.0 commands.

matteodepalo force-pushed the clean-up-old-code branch 4 times, most recently from 98d6641 to 7f43432 Compare October 12, 2023 09:17

Remove unified deployments code

3b2a533

matteodepalo force-pushed the clean-up-old-code branch 2 times, most recently from 2c94a65 to a35a74e Compare October 16, 2023 12:20

matteodepalo force-pushed the clean-up-old-code branch from a35a74e to 28d0300 Compare October 31, 2023 10:25

Fix tests after removing unified deployments beta flag

87862af

matteodepalo force-pushed the clean-up-old-code branch from 28d0300 to 87862af Compare October 31, 2023 10:31

matteodepalo added 2 commits October 31, 2023 10:42

Merge remote-tracking branch 'origin/main' into clean-up-old-code

472b743

Remove unifiedDeployment code after merging main

51b06b3

Remove functions from tests of fetchAppExtensionRegistrations

3a98250

matteodepalo marked this pull request as ready for review October 31, 2023 13:45

matteodepalo requested review from mattnoakes, alvaro-shopify and gonzaloriestra October 31, 2023 16:10

gonzaloriestra approved these changes Oct 31, 2023

View reviewed changes

packages/app/src/cli/api/graphql/all_app_extension_registrations.ts Show resolved Hide resolved

packages/app/src/cli/services/dev/processes/draftable-extension.ts Outdated Show resolved Hide resolved

packages/app/src/cli/services/deploy.ts Show resolved Hide resolved

mattnoakes approved these changes Oct 31, 2023

View reviewed changes

matteodepalo added 4 commits November 1, 2023 15:03

Fix draftable extensions filter

ba0360e

Remove unused code

c99d395

Add test cases for release=false scenarios

57582f9

Remove code related to usingExtensionsFramework and functions

59c81b6

mattnoakes approved these changes Nov 1, 2023

View reviewed changes

packages/app/src/cli/services/extensions/common.ts Show resolved Hide resolved

packages/app/src/cli/services/context/identifiers.test.ts Outdated Show resolved Hide resolved

isaacroldan reviewed Nov 2, 2023

View reviewed changes

packages/app/src/cli/models/extensions/extension-instance.ts Outdated Show resolved Hide resolved

isaacroldan reviewed Nov 2, 2023

View reviewed changes

packages/app/src/cli/models/extensions/extension-instance.ts Outdated Show resolved Hide resolved

Reintroduce isDraftable and update extension instance

5213fb7

tests

Revert "Remove code related to usingExtensionsFramework and functions"

73c924d

This reverts commit 59c81b6.

mattnoakes approved these changes Nov 3, 2023

View reviewed changes

matteodepalo added this pull request to the merge queue Nov 3, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 3, 2023

matteodepalo added this pull request to the merge queue Nov 6, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 6, 2023

matteodepalo added 2 commits November 6, 2023 14:21

Merge remote-tracking branch 'origin/main' into clean-up-old-code

f00875c

Remove SHOPIFY_CLI_UNIFIED_DEPLOYMENT from ruby files

b49cce5

matteodepalo force-pushed the clean-up-old-code branch from d656f30 to b49cce5 Compare November 6, 2023 14:58

matteodepalo enabled auto-merge November 6, 2023 14:58

matteodepalo added this pull request to the merge queue Nov 6, 2023

Merged via the queue into main with commit 29166a9 Nov 6, 2023

matteodepalo deleted the clean-up-old-code branch November 6, 2023 15:14

shopify-shipit bot temporarily deployed to nightly November 7, 2023 10:47 Inactive

shopify-shipit bot temporarily deployed to experimental November 10, 2023 12:15 Inactive

shopify-shipit bot temporarily deployed to production November 13, 2023 14:54 Inactive

shopify-shipit bot temporarily deployed to stable_3_51 November 23, 2023 09:39 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove unified deployments beta flag code #2936

Remove unified deployments beta flag code #2936

matteodepalo commented Oct 6, 2023 •

edited

Loading

github-actions bot commented Oct 6, 2023

github-actions bot commented Oct 31, 2023 •

edited

Loading

github-actions bot commented Oct 31, 2023

gonzaloriestra left a comment

mattnoakes left a comment

mattnoakes Oct 31, 2023

matteodepalo commented Nov 1, 2023

mattnoakes left a comment

matteodepalo commented Nov 2, 2023

matteodepalo commented Nov 2, 2023

mattnoakes left a comment

mattnoakes Nov 2, 2023

matteodepalo Nov 3, 2023

github-actions bot commented Nov 6, 2023

	const LOCAL_APP = (uiExtensions: ExtensionInstance[], functionExtensions: ExtensionInstance[] = []): AppInterface => {
	const LOCAL_APP = (uiExtensions: ExtensionInstance[]): AppInterface => {

Remove unified deployments beta flag code #2936

Remove unified deployments beta flag code #2936

Conversation

matteodepalo commented Oct 6, 2023 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

github-actions bot commented Oct 6, 2023

github-actions bot commented Oct 31, 2023 • edited Loading

Coverage report

Test suite run success

github-actions bot commented Oct 31, 2023

gonzaloriestra left a comment

Choose a reason for hiding this comment

mattnoakes left a comment

Choose a reason for hiding this comment

mattnoakes Oct 31, 2023

Choose a reason for hiding this comment

matteodepalo commented Nov 1, 2023

mattnoakes left a comment

Choose a reason for hiding this comment

matteodepalo commented Nov 2, 2023

matteodepalo commented Nov 2, 2023

mattnoakes left a comment

Choose a reason for hiding this comment

mattnoakes Nov 2, 2023

Choose a reason for hiding this comment

matteodepalo Nov 3, 2023

Choose a reason for hiding this comment

github-actions bot commented Nov 6, 2023

Differences in type declarations

New type declarations

Existing type declarations

matteodepalo commented Oct 6, 2023 •

edited

Loading

github-actions bot commented Oct 31, 2023 •

edited

Loading