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

Update peer dependency on Electron #2228

Merged
merged 2 commits into from
Sep 19, 2021
Merged

Update peer dependency on Electron #2228

merged 2 commits into from
Sep 19, 2021

Conversation

Jelmerro
Copy link
Contributor

This will hopefully make the dependency less strict for new stable and beta versions, so they can be installed without --force.

This will hopefully make the dependency less strict for new stable and beta versions, so they can be installed without --force.
@Jelmerro Jelmerro requested a review from remusao as a code owner September 18, 2021 20:01
@@ -34,7 +34,7 @@
"test": "nyc mocha --config ../../.mocharc.js"
},
"peerDependencies": {
"electron": "11.x || 12.x || 13.x"
"electron": "^11.0.0"
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 this needs to be either * or x, otherwise it will not allow major version updates.

Suggested change
"electron": "^11.0.0"
"electron": "*"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just check this on https://semver.npmjs.com/ and you are correct, so I experimented with the syntax, and it turns out we can do >11 to specify that everything starting at 11 is okay, including major versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I used >11 instead of >10, which is in line with current, is that Electron 11 has reached end of life on the 31st of August.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to clarify, is the intent to also include 11 or to exclude it and only support from 12 onward? If the former, then should it be >=11?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intent here is to exclude 11, and specify it to mean 12 and above, so we could also use >=12. Or in case you still want to support 11, it's indeed correct to use >=11. Which version has your preference?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Both are ok, I just wanted to confirm my understanding. I thought from the previous comment that you meant to exclude 10 but include 11. Thank you!

@remusao remusao added the PR: Polish 💅 Increment patch version when merged label Sep 19, 2021
@remusao remusao merged commit 8656b1a into ghostery:master Sep 19, 2021
@remusao
Copy link
Collaborator

remusao commented Sep 19, 2021

Thank you @Jelmerro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Polish 💅 Increment patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants