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

deps: update deps #5411

Merged
merged 9 commits into from
Apr 7, 2021
Merged

deps: update deps #5411

merged 9 commits into from
Apr 7, 2021

Conversation

bl-ue
Copy link
Contributor

@bl-ue bl-ue commented Mar 10, 2021

@bl-ue bl-ue added the tooling Helper tools, scripts and automated processes. label Mar 10, 2021
@bl-ue bl-ue requested review from owenvoke and mebeim March 10, 2021 20:37
@bl-ue bl-ue marked this pull request as draft March 10, 2021 20:37
@bl-ue
Copy link
Contributor Author

bl-ue commented Mar 10, 2021

We should enable Dependabot on this repo, like we do in the node client's repo.

Copy link
Member

@owenvoke owenvoke left a comment

Choose a reason for hiding this comment

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

LGTM! I agree about enabling Dependabot. Feel free to do that if you want (or I can if you'd like?). I'd suggest going with weekly updates to reduce the number of PRs we'd get from it.

@bl-ue
Copy link
Contributor Author

bl-ue commented Mar 10, 2021

Will you please do it? I'm not an org owner (yet), so I can't edit the repository in that type of way.

@bl-ue
Copy link
Contributor Author

bl-ue commented Mar 10, 2021

We should probably do it after this PR is merged though, because it might open duplicate PRs :)

@bl-ue bl-ue requested a review from owenvoke March 10, 2021 22:27
@bl-ue
Copy link
Contributor Author

bl-ue commented Mar 10, 2021

Please see the latest commit. I ran npx husky-4-to-5 to convert the old format to the new one.

@bl-ue
Copy link
Contributor Author

bl-ue commented Mar 11, 2021

Hmm...npm ci is failing locally as well as in the CI!

@bl-ue
Copy link
Contributor Author

bl-ue commented Mar 11, 2021

Not sure what changed to cause this error, but I fixed it by adding the name key to package.json. The full error was:

TypeError: Cannot read property 'length' of undefined
    at _incorrectWorkingDirectory (~/.npm/node_modules/lib/node_modules/npm/node_modules/npm-lifecycle/index.js:114:60)
    at ~/.npm/node_modules/lib/node_modules/npm/node_modules/npm-lifecycle/index.js:86:44
    at ~/.npm/node_modules/lib/node_modules/npm/node_modules/npm-lifecycle/index.js:218:12
    at callback (~/.npm/node_modules/lib/node_modules/npm/node_modules/graceful-fs/polyfills.js:295:20)
    at FSReqCallback.oncomplete (fs.js:165:5)

the code at that path (npm-lifecycle/index.js:114:60):

function _incorrectWorkingDirectory (wd, pkg) {
  return wd.lastIndexOf(pkg.name) !== wd.length - pkg.name.length
}

after a console.log(pkg), I found it to be the parsed contents of package.json.

So, I added name. It really should've always been there, because package.json isn't much of a real package.json otherwise. It also needs version, because npm displays the package as tldr-pages@undefined 😄, coincides with #5377 (comment).

@bl-ue bl-ue marked this pull request as ready for review March 16, 2021 14:20
@mebeim
Copy link
Member

mebeim commented Mar 17, 2021

Is the new .husky/pre-commit file required for Husky v5? Just want to make sure.

@bl-ue
Copy link
Contributor Author

bl-ue commented Mar 17, 2021

Yes, it is. You can see they have one, here. Also, I used this package to convert from version 2 to 5. It created a .gitignore and added some items, but it didn't add that, I'd say on purpose.

package.json Outdated Show resolved Hide resolved
Copy link
Member

@sbrl sbrl left a comment

Choose a reason for hiding this comment

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

Looks ok to me

What's husky, out of curiosity? Something that our GitHub Actions uses?

@navarroaxel
Copy link
Member

What's husky, out of curiosity? Something that our GitHub Actions uses?

@sbrl an easy wrapper to run hooks like eslint before pre-commit or pre-push, etc. Without deal with bash files in the .git/hooks directory. Just config in the package.json.

@bl-ue bl-ue requested a review from navarroaxel March 28, 2021 18:55
@bl-ue
Copy link
Contributor Author

bl-ue commented Mar 28, 2021

I just updated husky 5.1.35.2.0, and I upgraded the lockfile format from v1 (npm 6 and lower) to v2 (npm 7) as it's quickly becoming the norm (and we're already using it in the linter).

@bl-ue
Copy link
Contributor Author

bl-ue commented Mar 28, 2021

@owenvoke are we ready to merge?

@Waples
Copy link
Member

Waples commented Apr 7, 2021

guess i'll bump @owenvoke

@bl-ue
Copy link
Contributor Author

bl-ue commented Apr 7, 2021

Actually, they just released husky 6.0.0 so I need to make the change...

@bl-ue bl-ue marked this pull request as draft April 7, 2021 14:06
@bl-ue bl-ue marked this pull request as ready for review April 7, 2021 14:07
@bl-ue bl-ue merged commit 72a714b into master Apr 7, 2021
@bl-ue bl-ue deleted the bl-ue/update-deps-031021 branch April 7, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling Helper tools, scripts and automated processes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants