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

chore: bump fs-extra from 10.0.0 to 11.2.0 #321

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

threema-danilo
Copy link

No description provided.

@threema-danilo threema-danilo requested a review from a team as a code owner May 31, 2024 09:00
Copy link
Member

@erickzhao erickzhao left a comment

Choose a reason for hiding this comment

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

Hey @threema-danilo, we generally don't accept dependency bump PRs from outside contributors.

Is there a particular reason that you wanted to see this dependency bump (along with similar PRs in different repos)? I don't see a description in any of the other PRs.

electron/notarize#198
electron/fuses#44
electron/rebuild#1139

@threema-danilo
Copy link
Author

threema-danilo commented Jun 3, 2024

@erickzhao hi! Yes, the reason is that we were having issues with the NodeJS version resolution algorithm in combination with local file: dependencies. For some reason, NodeJS would resolve fs-extra to a different version than the one required by our local library.

Upon closer inspection I noticed that there were about 4 different versions of fs-extra included in the various electron libs. I hoped that bringing the versions more in line would help.

we generally don't accept dependency bump PRs from outside contributors

That's fine, but I'd like to note that some of the libraries still contain very old dependencies, like TypeScript 4.0 in electron/rebuild. Some libs also declare compatibility with Node 10, where LTS security support ended more than 3 years ago. Some maintenance there would make non-dependency-bump contributions easier.

@erickzhao
Copy link
Member

Thanks for the clarification! Will bring this up to other maintainers as well and hopefully have some forward momentum on this. :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants