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 deprecated --worker CLI flag #2603

Merged
merged 7 commits into from
Oct 24, 2024
Merged

Conversation

rbshop
Copy link
Contributor

@rbshop rbshop commented Oct 14, 2024

WHY are these changes introduced?

This flag has been deprecated for a while and is due to be removed with the next release

WHAT is this pull request doing?

  • Remove deprecated (and unused) --worker flag

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

Copy link
Contributor

shopify bot commented Oct 14, 2024

Oxygen deployed a preview of your rb-remove-cli-flag-deprecations branch. Details:

Storefront Status Preview link Deployment details Last update (UTC)
sitemap ✅ Successful (Logs) Preview deployment Inspect deployment October 24, 2024 5:47 PM
third-party-queries-caching ✅ Successful (Logs) Preview deployment Inspect deployment October 24, 2024 5:47 PM
classic-remix ✅ Successful (Logs) Preview deployment Inspect deployment October 24, 2024 5:47 PM
Skeleton (skeleton.hydrogen.shop) ✅ Successful (Logs) Preview deployment Inspect deployment October 24, 2024 5:47 PM
custom-cart-method ✅ Successful (Logs) Preview deployment Inspect deployment October 24, 2024 5:47 PM
metaobjects ✅ Successful (Logs) Preview deployment Inspect deployment October 24, 2024 5:47 PM

Learn more about Hydrogen's GitHub integration.

@@ -82,7 +81,7 @@ export default class Deploy extends Command {
}),
preview: Flags.boolean({
description:
'Deploys to the Preview environment. Overrides --env-branch and Git metadata.',
'Deploys to the Preview environment. Overrides Git metadata.',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This previously stated that it Overrides --env-branch, but looking at the original code in this file a little further down it appears that this wasn't actually the case (and I think we should also remove the claim that this overrides the Git metadata too?)

Copy link
Contributor

Choose a reason for hiding this comment

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

yes - remove --env-branch comment

overrides Git metadata - @frandiox Does hydrogen deploy preview actually overrides git metadata? I don't see its doing that in code either. Running npx shopify hydrogen deploy --preview just creates a preview deploy with no changes to user's repo (which is expected). I think the metadata here is referring to oxygen specific metadata.

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've reverted the removal of the flag, but I think this update should remain as the code shows that this claim was false anyway

@rbshop rbshop added the 2024.10 label Oct 18, 2024
@@ -0,0 +1,5 @@
---
'@shopify/cli-hydrogen': patch
Copy link
Contributor

Choose a reason for hiding this comment

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

@frandiox Should this be a major bump?

Copy link
Contributor

Choose a reason for hiding this comment

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

Technically yes, but since we are releasing under the bundled global CLI, it doesn't matter much. It's not going to be a major bump in the bundled CLI yet, so feel free to do it in a minor here. It's also been announced as deprecated for very long so it's probably OK.

Copy link
Contributor

@wizardlyhel wizardlyhel left a comment

Choose a reason for hiding this comment

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

Gonna need to revert the changes to --env-branch and leave it in the deprecated state. It can only be remove once --env is supported in CI. Details https://github.com/Shopify/custom-storefronts/issues/560

@rbshop rbshop changed the title Remove deprecated CLI flags Remove deprecated --worker CLI flag Oct 24, 2024
@rbshop
Copy link
Contributor Author

rbshop commented Oct 24, 2024

I have now reverted the --env-branch flag and updated the PR description and title accordingly

@wizardlyhel wizardlyhel merged commit da42a2e into main Oct 24, 2024
13 checks passed
@wizardlyhel wizardlyhel deleted the rb-remove-cli-flag-deprecations branch October 24, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants