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.

Sign up for GitHub

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

Jump to bottom

89 husky install deprecated #90

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

89 husky install deprecated #90

wants to merge 2 commits into from

Conversation

GMSoudersJr
Copy link
Collaborator

Previously

image

Currently

image

@GMSoudersJr
chore(husky): remove deprecated lines
49bdec9
@GMSoudersJr
fix(husky): update README.md and script command
72caa52 
husky install is deprecated, the documentation also said to use
postinstall instead of prepare because yarn does not support it. Updated
the README.md to mirror the script change as well.

closes #89
@GMSoudersJr GMSoudersJr linked an issue Sep 11, 2024 that may be closed by this pull request
Husky install deprecated #89
Open
ivanms1
ivanms1 approved these changes Sep 23, 2024
View reviewed changes
Copy link
Collaborator

@ivanms1 ivanms1 left a comment

Choose a reason for hiding this comment

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

Thank you!
just a small suggestion and a question.

README.md
@@ -15,7 +15,7 @@ To view our site and see your contributions, visit <https://fcc-seoul.fly.dev>
- Husky setup (only once)

```bash
yarn prepare
yarn run postinstall
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you don't need run here, but not 100% sure.

Copy link
Collaborator Author

@GMSoudersJr GMSoudersJr Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

okay, neither am I. I was just following husky's docs.

image

After looking through the yarn docs:
image

Okay, I will get rid of it.

.husky/pre-commit
Comment on lines -1 to -3
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

Copy link
Collaborator

Choose a reason for hiding this comment

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

not opposed to it, but can I know the reason for removing this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure.

And there's more

image

This was another message that appeared when running husky. I originally posted this in the issue. I guess it is better to include it in the PR next time.

@GMSoudersJr
Copy link
Collaborator Author

GMSoudersJr commented Sep 25, 2024 •
edited
Loading

🤔 Is yarn postinstall needed in README.md anymore?

After reading more of the docs, I came across this: postinstall will run directly after install. No further commands needed.

image

I ran yarn, which equates to yarn install and well, here's the output:

image

I thought it might be because I already have the project and ran husky before. So, I:

  1. Created a new directory
  2. Cloned this repo into it
  3. Went into that directory
  4. git switch this branch
  5. yarn then...
    image

Changed some code and the linter ran as usual. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivanms1 ivanms1 ivanms1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Husky install deprecated
2 participants
@GMSoudersJr @ivanms1