Skip to content

Conversation

@lillialexis
Copy link
Contributor

@lillialexis lillialexis commented May 8, 2025

Summary:

See the Slack thread here for more context. Basically, installing the pre-push linter should be default off in most contexts other than webapp, so this PR toggles that.

Related PRs:

Issue: FEI-6625

Test plan:

  • Run ka-clone --repair in a repo, see that the hook ISN'T installed in .git/hooks
  • Run ka-clone --repair --pre-push-lint in a repo, see that the hook IS installed in .git/hooks

Copy link
Member

@csilvers csilvers left a comment

Choose a reason for hiding this comment

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

I know everyone else is happy about this, but I am sad. :.(

@lillialexis lillialexis merged commit 7c66ee6 into master May 27, 2025
lillialexis added a commit to Khan/khan-dotfiles that referenced this pull request May 27, 2025
…om true to false (#145)

## Summary:
[See the Slack thread here for more context](https://khanacademy.slack.com/archives/C06P903SXV3/p1746624684972699). 

For the `frontend` repo, and for most other repos other than webapp, we have chosen not to use the `pre-push.lint` hook. In [this PR](Khan/ka-clone#23) I togged the default from `true` to `false`. So now we have to explicitly add it for webapp when setting up a computer.

Related PRs:
- #145 👈 you are here
- Khan/ka-clone#23
- Khan/ka-clone#24
- https://github.com/Khan/our-lovely-cli/pull/1000
- https://github.com/Khan/our-lovely-cli/pull/1001

Issue: FEI-6625

## Test plan:
I actually have no idea how to test this!

Author: lillialexis

Reviewers: csilvers, jeresig

Required Reviewers:

Approved By: csilvers, jeresig

Checks:

Pull Request URL: #145
lillialexis added a commit that referenced this pull request May 27, 2025
## Summary:
[See the Slack thread here for more context](https://khanacademy.slack.com/archives/C06P903SXV3/p1746624684972699).

Before, to see if we are on the right version of `ka-clone`, Repo Rangers would check if the flags it needed were present. If they weren't, rrs would just need to update ka-clone to the latest version.

But now we are making a change to ka-clone that end up *not changing* any of ka-clone's flags (it just toggles a default), so for this change, we can no longer rely on the presence/absence of flags to determine if ka-clone is up to date. Instead, we will *also* need use a --version flag to check the version of ka-clone.

Related PRs:
- Khan/khan-dotfiles#145
- #23
- #24 👈 you are here
- https://github.com/Khan/our-lovely-cli/pull/1000
- https://github.com/Khan/our-lovely-cli/pull/1001

Issue: FEI-6625

## Test plan:
Run `ka-clone --version` and see `1`

Author: lillialexis

Reviewers: csilvers, jeresig

Required Reviewers:

Approved By: csilvers, jeresig

Checks:

Pull Request URL: #24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants