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

Change the defaults of ka-clone and reorder things slightly #15

Open
wants to merge 2 commits into
base: feature/rrangers
Choose a base branch
from

Conversation

lillialexis
Copy link
Member

@lillialexis lillialexis commented Nov 21, 2024

Summary:

I also added some "opposite" args in arg groups (e.g., --protect-master and --no-protect-master), which users can't use at the same time.

So, the defaults are now (currently):

Option               | Is Default?
---------------------+-----------
use ka email         | yes
protect master hooks | yes
branch name hook     | yes
lint commit msg hook | no
pre-push lint hook   | yes
git config linked    | yes
commit template      | yes

@csilvers are these defaults good? Do you think I should toggle any of them? This is what I landed on after I started the implementation.

Should I rename these args to be a little more consistent with each other and their function? I don't love how they currently are (e.g., --no-msg meaning "don't use the ka commit template" and --no-lint referring to the pre-push linter but not the commit-msg linter), but don't know who else may have hard-coded them as they currently are. (Or if it's worth doing.)

Issue: FEI-6001

Test plan:

  • Run ka-clone with no args, sees that the defaults are what we are saying they are
  • Run with conflicting arguments (e.g., --protect-master and --no-protect-master), and see that it errors
  • Try running with all arguments that are opposite of the deep faults and see that they do what we expect them to do

@lillialexis lillialexis requested a review from a team November 25, 2024 22:59
@lillialexis lillialexis marked this pull request as ready for review November 25, 2024 22:59
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