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 CLI flags #2603

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

Conversation

rbshop
Copy link

@rbshop rbshop commented Oct 14, 2024

WHY are these changes introduced?

Both flags have been deprecated for a while and are due to be removed with the next release

WHAT is this pull request doing?

  • Remove deprecated (and unused) --worker flag
  • Remove deprecated --env-branch flag and associated code

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 14, 202411:55 AM
third-party-queries-caching ✅ Successful (Logs) Preview deployment Inspect deployment October 14, 202411:55 AM
classic-remix ✅ Successful (Logs) Preview deployment Inspect deployment October 14, 202411:55 AM
Skeleton (skeleton.hydrogen.shop) ✅ Successful (Logs) Preview deployment Inspect deployment October 14, 202411:55 AM
custom-cart-method ✅ Successful (Logs) Preview deployment Inspect deployment October 14, 202411:55 AM
metaobjects ✅ Successful (Logs) Preview deployment Inspect deployment October 14, 202411:55 AM

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
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?)

@@ -357,10 +354,6 @@ export async function runDeploy(
);
}

if (isCI && envBranch) {
Copy link
Author

Choose a reason for hiding this comment

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

This was added in #2281, with the note:

Future task
env flag will require more effort as it needs to be available on our dependant services
the current fix only patches it for the envBranch flag

Do we still want this feature? Is that future work tracked anywhere?

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.

1 participant