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

Update dependencies. #133

Merged
merged 4 commits into from
Feb 9, 2023
Merged

Update dependencies. #133

merged 4 commits into from
Feb 9, 2023

Conversation

sunoru
Copy link
Contributor

@sunoru sunoru commented Feb 8, 2023 •
edited
Loading

The vulnerabilities warnings and deprecation warnings are fixed.

@sunoru
Use fs.rmSync instead of fs.rmdirSync.
1d39f8e 
To get rid of the deprecation warning. julia-actions#119
@sunoru sunoru requested a review from a team as a code owner February 8, 2023 18:54
@sunoru sunoru mentioned this pull request Feb 8, 2023
Closed
@SaschaMann
Copy link
Member

Doesn't seem to be as straightforward as the deprecation warning suggests: src/setup-julia.ts(78,16): error TS2339: Property 'rmSync' does not exist on type 'typeof import("fs")'.

@sunoru
Copy link
Contributor Author

sunoru commented Feb 9, 2023

Doesn't seem to be as straightforward as the deprecation warning suggests: src/setup-julia.ts(78,16): error TS2339: Property 'rmSync' does not exist on type 'typeof import("fs")'.

It is because the version of @types/node (v12) is too old but we are actually using node v16 on the ci

@sunoru
Copy link
Contributor Author

sunoru commented Feb 9, 2023

I would like to turn this PR into bumping the dependency versions since many of them are too old and have vulnerabilities. Is that OK?

@sunoru
Copy link
Contributor Author

sunoru commented Feb 9, 2023

And I don't understand why this was needed... #112

@sunoru sunoru changed the title Use fs.rmSync instead of fs.rmdirSync. Update dependencies. Feb 9, 2023
@SaschaMann
Copy link
Member

And I don't understand why this was needed... #112

Because without it, npm install didn't work and this was the suggested fix in the error message.

SaschaMann
SaschaMann approved these changes Feb 9, 2023
View reviewed changes
Copy link
Member

@SaschaMann SaschaMann left a comment

Choose a reason for hiding this comment

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

Thanks for fixing all of this :)

@SaschaMann SaschaMann enabled auto-merge (squash) February 9, 2023 22:56
@SaschaMann SaschaMann merged commit 152b85e into julia-actions:master Feb 9, 2023
@sunoru sunoru deleted the patch-1 branch February 9, 2023 23:16
@sunoru
Copy link
Contributor Author

sunoru commented Feb 9, 2023

Nice, thank you for merging it!

And I don't understand why this was needed... #112

Because without it, npm install didn't work and this was the suggested fix in the error message.

Using --legacy-peer-deps would ignore a lot of dependency issues and I think it should be avoided 😂 (so I fixed them in this pr)

@sunoru
Copy link
Contributor Author

sunoru commented Feb 9, 2023

BTW #119 can also be closed.

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

@SaschaMann SaschaMann SaschaMann 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.
2 participants
@sunoru @SaschaMann