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

Electron w/ Chrome 73: 5-0-x or 6-0-x #16876

Closed
groundwater opened this issue Feb 11, 2019 · 8 comments
Closed

Electron w/ Chrome 73: 5-0-x or 6-0-x #16876

groundwater opened this issue Feb 11, 2019 · 8 comments

Comments

@groundwater
Copy link
Contributor

Node.js has announced Node v12 will target V8 7.3, which means Chrome 73. Our current 5-0-x beta line contains Chromium 72 and node's master branch. Our intention was that node 12 would be ready for release around the time our 5-0-x line was going stable. Given node's announcement, this means Electron's 5.0.0 stable would eventually contain a non-official version of node with V8 7.2.

We have decided it's worth having an officially release node, and thus want to push our focus towards an Electron with Chrome 73.

What we haven't decided is whether it's better to push the upgrade into 5-0-x or 6-0-x. The latter would immediately deprecate the 5-0-x release line. There would be no 5.0.0 stable. Both options have pros and cons. Strictly by our versioning document, this must be 6-0-x. The 5-0-x line however is brand new, and likely has no apps in production on it. Whatever we decide, it will warrant a blog post explaining the change.

We have opted for a short debate to entertain both possibilities, but will come to a conclusion by next week.

In this corner 👈 of the room, we have @nornagon advocating to merge 73 into 5-0-x.

In the other corner 👉 , we have @felixrieseberg advocating issuing a 6-0-x release.

Please note that ☝️ were kind enough to volunteer a writeup. This is one of those dicey issues, so I'd like to remind folks that personal attacks will result in an immediate ban from all Electron organizations.

This issue will be locked until both @nornagon and @felixrieseberg have added their reply.

@electron electron locked and limited conversation to collaborators Feb 11, 2019
@groundwater groundwater pinned this issue Feb 11, 2019
@felixrieseberg
Copy link
Member

felixrieseberg commented Feb 11, 2019

Before I start arguing for making this change in the 6-0-x branch, I’d want to appreciate the hard work that’s gone into closing the distance between Electron and Chrome & Node.js. The problem we’re having here is a good one for Electron as a platform that companies and developers can rely on, because it increases security and stability. In the end, it allows Electron to be more deserving of developer’s trust.

I do not believe that we should upgrade the major version Chrome in the 5-0-x branch.

1️⃣ Upgrading the major version goes directly against our own versioning document, which is supposed to be one of the few pillars of predictability and stability in the project.

2️⃣ Upgrading the major Chrome version isn’t a minor change. It doesn’t just break our versioning policy in the letter, it also breaks it in spirit. We all know that the code delta between Chrome versions is massive and comes with numerous changes in behavior, some of them breaking changes that are now “enabled by default”. Chrome 73 in particular is a rather large release.

3️⃣ It’d break NODE_MODULE_VERSION stability: Ever since we’ve introduced our versioning policy, all releases in one major release shared the same NODE_MODULE_VERSION, with 4.0.4 being the only exception. The more exceptions we need to add to this list, the more complexity we’re introducing into this process. 5-0-x is currently on 68, upgrading in 5-0-x would bump it to 70. We never made this a hard rule, but breaking with Node.js in that regard would be yet another thing we’d need to explain to users – and we don’t have the best track record when it comes to explaining native addons to our community.

4️⃣ It’d change the definition of beta for Electron. For most users, Electron is “just” Chrome and Node.js mushed together – if a beta doesn’t even give you a preview to the “right” version of Chrome, then it’s for all intents and purposes a pretty shaky alpha. There are many definitions of what beta is supposed to mean, but those who learn from Wikipedia would expect a release that “… is feature complete but likely to contain a number of known or unknown bugs.”

I fully understand why we need to update Chrome. I'm aware that not upgrading Chrome in 5-0-x likely means that 5-0-x is dead on arrival. I find the above four points powerful enough to consider an immediately deprecated release line the better compromise.

Thoughts about the future

As a future consideration, we may introduce one or both of the following: alpha releases that have looser stability constraints to betas; for example it would be allowable to admit new features while a stability channel is in alpha
– electronjs.org/docs/tutorial/electron-versioning

If we cannot afford to declare a branch to be as stable as we want to, we should make use of the alpha tag. It’s widely recognized in the industry as an earlier stage in development where fundamentals can still shift around – even things as fundamental as the major Chrome version.

@nornagon
Copy link
Member

Upgrading to M73 in the 5.0.0-beta line does violate the letter of the versioning policy document, but I don't think it violates the spirit. I'd argue that we made a mistake in cutting 5-0-x before we knew what version of V8 node would release with, and that upgrading to M73 is the best way to fix that mistake. It's early enough in the beta cycle that the impact on developers here is minimal. Source: I'm the one who's making test builds of Slack on Electron 5, and I'm not worried about M73 landing in 5-0-x. We haven't yet done any significant testing that would be invalidated by changing Chrome version.

The confusion of changing Chromium version in 5.0.0-beta.3 is a one-off confusion. Once it's done, we can safely tell people "Electron 5.x is based on Chromium 73". There's not much likelihood that people will be regularly testing on beta.1 or beta.2 once we're past beta.3, and it's likely that most people won't even notice that 5.x was ever on anything but M73. But skipping the entire 5.x release line is something we will have to continually explain for the next six months, in conversations about supported versions, in bugs where we tell people to upgrade from 4 to 6, and in meetings with each other or with internal teams at companies using Electron.

Chromium 73 is about as big as other Chromium releases; I don't think there's any reason to be especially concerned about this particular upgrade. The feature list on chromestatus.com is long, but it's long for every release, and I don't see anything particularly concerning in that list. There are some changes with the potential for bugs in the M72->M73 upgrade, but 5.x is already a 3-version bump from 4.x, and this bump feels no more dangerous than the other ones, and we're still early enough in the beta cycle that we will be able to catch any serious issues long before stable.

Regarding NODE_MODULE_VERSION, we should not be matching node.js's node module version anyway—it changed in 4.0.4 so that we wouldn't collide, and we are going to need to do the same in 5.x anyway. It should not be 68 currently; that's a bug. See here for details & links.

@groundwater
Copy link
Contributor Author

Non-binding voting is open. Please 👍 your preferred approach in the issue. It is unnecessary to 👎. Let's stick to happy-thoughts.

Replies are welcomed to expand on your vote, but we will minimize comments as off-topic if they could be reasonably summarized as a 👍.

@githoniel

This comment has been minimized.

@malept
Copy link
Member

malept commented Feb 12, 2019

@githoniel 4.0.4 was used as an example. The short version is that the NODE_MODULE_VERSION was incorrect between Electron 4.0.0-beta.1 and 4.0.3, Electron ^4.0.4 has the correct one. For more details, start at #16687 and read through all of the related issues.

The Electron 5 series will have a different NODE_MODULE_VERSION than ^4.0.4 (which may or may not change within the beta series, depending on the outcome of this issue).

@codebytere
Copy link
Member

codebytere commented Feb 14, 2019

We've decided to move forward, based on feedback and above sentiment, with the choice to merge Chromium 73 into the 5-0-x release branch. The upgrade will be merged into the branch within a day or so, and the next beta of 5-0-x, 5.0.0-beta.3, will have M73. Thank you all for your input!

This issue will be closed as soon as the merge takes place.

cc @electron/wg-releases @electron/wg-upgrades

@tommoor
Copy link
Contributor

tommoor commented Feb 15, 2019

I think this process / thread was a fantastic example of open source development and community engagement done well – special thanks to @felixrieseberg @nornagon for taking the time to write up persuasive arguments for both sides of the coin! 🙏

@MarshallOfSound
Copy link
Member

The PR that merges Chromium 73 into 5-0-x has been merged --> #16975

The change has been released in 5.0.0-beta.3, closing this one out now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants