tiktok / sparo

Sparo optimizes performance of Git operations for your large frontend monorepo.
https://tiktok.github.io/sparo/
MIT License
162 stars 8 forks source link

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

Closed jzhang026 closed 5 months ago

jzhang026 commented 6 months ago

Basic Checks

Have you run rush change for this change?

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

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

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

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

Summary

Detail

How to test it

chengcyber commented 6 months ago

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 ".

jzhang026 commented 6 months ago

@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 ".

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.