Skip to content

Add argument for using deprecated starter code#33

Merged
alexguo8 merged 2 commits intomainfrom
alex/add-deprecated-argument
Mar 13, 2022
Merged

Add argument for using deprecated starter code#33
alexguo8 merged 2 commits intomainfrom
alex/add-deprecated-argument

Conversation

@alexguo8
Copy link
Contributor

@alexguo8 alexguo8 commented Mar 5, 2022

Notion ticket link

Implementation description

  • If deprecated flag is supplied, warn the user, prompt for auth and file storage options and clone from release branch
  • If deprecated flag is not supplied, do not prompt for auth and file storage options and clone from release-v2 branch
  • Since --auth and --fs flags are ways to opt-in (ie. in deprecated version, leaving them out means we prompt for it as opposed to opt-out), these flags are still accepted when deprecated is false, it will just not do anything since they are automatically opt-in anyways
  • Confirmation message still tells the user that they are using built-in auth and built-in file storage
  • Testing flag still works for local version of starter code - the deprecated flag does affect which prompts are shown though

Steps to test

image

  1. Run yarn dev --de and verify that you get a warning and prompt for auth/file storage.

image

  1. Run yarn dev and verify that you do not get prompts for auth/file storage.

image

What should reviewers focus on?

  • Any wording changes?

Checklist

  • My PR name is descriptive and in imperative tense
  • My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
  • I have run the appropriate linter(s)
  • I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR
  • I have updated the version number in package.json according to Semantic Versioning specs (semver) if I will be publishing these changes to the npm registry

sherryhli
sherryhli previously approved these changes Mar 6, 2022
Copy link
Member

@sherryhli sherryhli left a comment

Choose a reason for hiding this comment

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

Any wording changes?

Nah, looks good to me! 👍

I think we can go ahead and bump the package version, the ongoing work on release-v2 should not have any impact

Copy link
Member

@sherryhli sherryhli left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

@alexguo8 alexguo8 merged commit 7c0cd88 into main Mar 13, 2022
@alexguo8 alexguo8 deleted the alex/add-deprecated-argument branch March 13, 2022 15:22
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