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

feat: Sparo checkout support --to/--from option for direct projects selection #67

Merged
merged 5 commits into from
May 9, 2024

Conversation

jzhang026
Copy link
Contributor

Basic Checks

Have you run rush change for this change?

  • Yes
  • No

If No, please run rush change before, this is necessary.

If adding a new feature, the PR's description includes:

  • Reason for adding this feature
    Instead of creating Sparo Profiles, sometimes I just want to checkout some projects randomly based on RushJS selector.
  • How to use
    sparo checkout --to project-A project-B --from project-C project-D
  • A basic example

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

Summary

Detail

How to test it

@jzhang026 jzhang026 force-pushed the feat/checout-to-from-selector branch from c11e31f to 580e47b Compare April 29, 2024 12:32
@chengcyber
Copy link
Member

I think the most breaking change from this PR is mental model, which is how we treat "sparo profile"?

Before this PR: "profile" is the only core concept in sparo commands. All commands will take care of profile, which mentioned in the discussion. For example, "sparo checkout " will sync up the folders by the recorded profiles after checking out to the target branch to make everything consistency and prohibit footguns.
At the same time, "--to" and "--from" are only supported in "sparo-ci checkout", which means the flexibility is allowed in CI.

After this PR: "profile" is changed. It's now more like a convenience way to "--to" and "--from". It unveils powerful "--to" and "--from" and more free to let users decide how to use it. As you can tell, the power is a double-edge sword. It can bring easy-to-use story and it can also make it pretty error-prone.
The following design questions would be:

  1. Do we need to record "--to" and "--from" to make project folders correct consistency after running "sparo checkout", "sparo pull" ... If so, it's pretty hard to maintain comparing with "profile"...
  2. If we are recording "--to" and "--from", we need a way to clear them or reset them. Just like the idea of "--profile" resets the current profiles and "--add-profile" appends more profiles.
  3. We might also need the unimplemented command "sparo inspect" to help people understand the current state. "sparo inspect" can display both the recorded profiles and "--to" and "--from" parameters.

My proposal:
I do think some of our users could be an "advance" user which requests more freedom way to use "sparo" and they can take responsibility to make it work well. So, A much more balance implementation could be introducing a feature toggle to control the behavior of "sparo checkout". To be more specific, after running sparo config sparo.checkout advance, "sparo" now can allow the "--to" and "--from" parameter used in "sparo checkout". And we treat those "--to" and "--from" as a temporary state and doesn't make them into consideration when running "sparo checkout <new_branch>".

@jzhang026
Copy link
Contributor Author

jzhang026 commented Apr 30, 2024

@chengcyber thanks for your insights.

I think the most breaking change from this PR is mental model, which is how we treat "sparo profile"?

Sparo profile takes precedence at all times despite we introduced --to and --from. Sparo profile is still the core concept this tool following.

And we treat those "--to" and "--from" as a temporary state and doesn't make them into consideration when running "sparo checkout <new_branch>".

  • sparo checkout <new_branch> without any profile provided will keep last time --to and --from if there are any.
  • sparo checkout --profile <branch> will just follow what stipulated in the profile without keeping any --to and --from selection of last checkout.

To put it simply, --to/--from selection is, to some sense, temporary and volatile, it will be cleared in a checkout as long as any profile related options provided.
Yes, Sparo Profile is still the core concept of this tool. We are adding --to/--from to expose some flexibility for some ad-hoc scenarios. By considering that, I don't quite agree the proposal of adding another config which may make it less handy.

@chengcyber chengcyber enabled auto-merge May 9, 2024 22:46
@chengcyber chengcyber merged commit 486822b into tiktok:main May 9, 2024
3 checks passed
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.

4 participants