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

Deprecations in 9.1.1 are breaking changes #1474

Closed
nimafallahian opened this issue Jul 22, 2024 · 7 comments
Closed

Deprecations in 9.1.1 are breaking changes #1474

nimafallahian opened this issue Jul 22, 2024 · 7 comments

Comments

@nimafallahian
Copy link

Wanted to inform that the deprecations made in version 9.1.1 are breaking changes.

#!/bin/sh

# shellcheck source=.husky/_/husky.sh
. "$(dirname "$0")/_/husky.sh"

Is not only deprecated but it's not working as well and is breaking the automated pipelines. Also, I don't like the idea of removing code with running husky and without using a --write tag. As a developer I do need to have full control on what's happening. I do appreciate the work done here, although this makes debugging a bit harder. :D

@typicode
Copy link
Owner

Hey,

v9.1.0 added deprecation message without removing code but it wasn't actually possible to make it work with the shell script refactor. So instead, I went with an automatic removal of code in v9.1.1.

While I sometimes prefer to manually update like you, I guess with this approach more hooks will be ready for v10. Sometimes people tend to leave deprecation messages for a long time without taking actions, so here at least it will be done and prevent frictions later.

When updating husky, prepare should run and automatically update hooks so no breaking. Not sure why it broke for you in the CI, though.

Thanks for feedback and sorry for the issue.

@melink14
Copy link

This also broke for my project (causing release to fail since git commit of tag and changelog failed`).

I don't understand what you mean by it should be fixed automatically. husky was updated by dependabot so if it was supposed to run then, any updated hooks would be completely ignored and never committed back to the repo.

I guess after that all runs of git commit will fail.

To clarify, is the manual work around to delete the deprecated lines ourselves?

melink14 added a commit to melink14/rikaikun that referenced this issue Jul 22, 2024
melink14 added a commit to melink14/rikaikun that referenced this issue Jul 22, 2024
@iowillhoit
Copy link

Hey @typicode, the change in 9.1.1 is also affecting our builds in Github Actions. Here is an example of it failing.

It looks like the regex that was written to clean this up is not quite flexible enough.

Here is the pre-commit file from the failing repo. You can see here (https://regex101.com/r/bZYGon/1) that the regex does not match what we have in our hook. Here is another pre-commit example that is slightly different but also does not match the regex.

This is going to break a lot of our repos' releases in the oclif and the Salesforce CLI orgs as they start rolling over to 9.1.1.

Can this be reverted and released in a v10 or could you release a new version with a slightly more flexible regex?
Thank you!

@typicode
Copy link
Owner

I see. I wrote a test for it, I wonder why the test passes but not on your CI:
https://github.com/typicode/husky/blob/main/test/12_rm_deprecated.sh#L8-L18

If it causes too much problems, I may just revert for now and publish v10 later.

@melink14 I mean that npm i should trigger "prepare": "husky". husky commands removes the deprecated lines.
This should happen before commit is running. So when it's running the deprecated lines shouldn't be there anymore. But I guess with dependabot it works differently.

@iowillhoit
Copy link

iowillhoit commented Jul 22, 2024

Yea, the regex (and test) would work if our files had the same "headers". Our files are slightly different so the regex does not match.

In our files: #!/bin/sh
Regex expects: #!/usr/bin/env sh

In our files: . "$(dirname $0)/_/husky.sh"
Regex expects: . "$(dirname -- "$0")/_/husky.sh"

I took a stab at updating the regex. It might not be perfect, but it matches a few variations that we have: https://regex101.com/r/WuObfs/1

@melink14
Copy link

It looks like the regex in my files are the same as iowillhoit's examples.

I also thought it could be because of some deprecation lines I saw:

> rikaikun@2.5.59 prepare
> husky install

install command is DEPRECATED

but it looks like the regex is the actual cause!

@typicode
Copy link
Owner

I reverted it, so v9.1.2 only displays a deprecation message and doesn't attempt to modify files.
Sorry for the trouble.

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

No branches or pull requests

4 participants