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

Release - 1.0 #2905

Merged
merged 25 commits into from
Jul 23, 2019
Merged

Release - 1.0 #2905

merged 25 commits into from
Jul 23, 2019

Conversation

nivida
Copy link
Contributor

@nivida nivida commented Jun 20, 2019

Description

This is the stable release of v1.0.

Compare View

v1.0.0-beta.37 -> v1.0.0

Type of change

  • Release

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no warnings.
  • I have updated or added types for all modules I've changed
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run test in the root folder with success and extended the tests to cover all changes.
  • I ran npm run build in the root folder and tested it in the browser and with node.
  • I have tested my code with an ethereum test network.

@nivida nivida added In Progress Currently being worked on Release 1.x 1.0 related issues labels Jun 20, 2019
@nivida nivida changed the base branch from 2.x to 1.x June 20, 2019 11:39
@coveralls
Copy link

coveralls commented Jun 20, 2019

Coverage Status

Coverage remained the same at 83.071% when pulling 963e5c5 on release/1.0 into fcaaca9 on 1.x.

Copy link
Contributor

@spalladino spalladino left a comment

Choose a reason for hiding this comment

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

Thank you so much for tackling this @nivida!! I'm super happy to see this happening.

I left two questions on dependencies changes. I'm worried about eventemitter3 in particular as we are shifting from 1.x to 3.x, but since I now little about that library, if you have already reviewed the changelog to make sure it's safe to update, I'm fine with it.

As for websockets, I'm glad we are moving away from the github version since it was incorrectly packed and was giving a lot of headaches to many developers when installing it.

packages/web3-providers-ws/package.json Outdated Show resolved Hide resolved
packages/web3-core-promievent/package.json Show resolved Hide resolved
@alcuadrado
Copy link

I'm surprised by the number of dependencies upgrades to new major versions of this PR, especially because none of their uses has changed.

Does this version of the library have enough tests to do these upgrades with enough confidence that nothing breaks, @nivida? I'm not familiar enough with this codebase yet.

I agree with @spalladino that the eventemitter3 is a special case, as it's exposed through promievents.

@sethfork
Copy link

Great to see the progress here @nivida! I'm curious what the 2.x branch/release is, as that seems to be a recent development.

As for websockets, I'm glad we are moving away from the github version since it was incorrectly packed and was giving a lot of headaches to many developers when installing it.

+1 @spalladino. Can't wait to remove my preinstall scripts removing websocket git folders.

@nivida
Copy link
Contributor Author

nivida commented Jun 25, 2019

Does this version of the library have enough tests to do these upgrades with enough confidence that nothing breaks, @nivida? I'm not familiar enough with this codebase yet.

Yes, I've downgraded the packages if the tests were failing.

I agree with @spalladino that the eventemitter3 is a special case, as it's exposed through promievents.

Upgrading of the EventEmitter package doesn't have an impact on the functionality of Web3.js but I will definitely do some additional tests here too.

@sethfork I will publish an announcement which will explain anything :-)

Edit: I've checked the release notes of the EventEmitter package and the only breaking change they had was on the release of 2.0.0 which doesn't have an impact on Web3.js. @spalladino @alcuadrado

@wbt
Copy link
Contributor

wbt commented Jul 2, 2019

Is there any particular reason this version was picked, including the security audit failure due to a tar dependency fixed in -beta38? Is it because beta37 is the last version to have had the pre-built Javascript files?

@wbt
Copy link
Contributor

wbt commented Jul 2, 2019

Cross-linking to comments by @sshelton76 on April 22++ for convenient reference.

@michaelsbradleyjr
Copy link
Contributor

michaelsbradleyjr commented Jul 12, 2019

@gnidan it seems to exist on this branch and with npm i [email protected] from the registry.

I raised the matter here because there is a release planned for tomorrow, wanted to make sure to raise awareness prior to release.

@gnidan
Copy link
Contributor

gnidan commented Jul 12, 2019

Ah, ok, in that case @michaelsbradleyjr: I vote that we don't fix additional bugs (bugs not introduced by this PR), since the primary motivator for this release is to enable further patching. I think instead we should just try to get PRs ready for 1.0.1 and the 2.0 alpha.

Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

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

PR trufflesuite/truffle#2182 reasonably demonstrates that this release passes Truffle's CI. Approving this since it meets acceptance criteria :)

Woot! Thanks @nivida!

@iurimatias
Copy link
Contributor

it's a 👍

1.0 here we come! Thanks @nivida

@nivida nivida removed the In Progress Currently being worked on label Jul 13, 2019
@nivida
Copy link
Contributor Author

nivida commented Jul 13, 2019

FYI

Bildschirmfoto 2019-07-13 um 13 12 18

@michaelsbradleyjr
Copy link
Contributor

michaelsbradleyjr commented Jul 16, 2019

There's now a fix in #2938 for the decrypt bug — see here and here. It still may be desirable to have that PR land in a future 1.x patch, but I wanted to give a heads up.

@levino
Copy link
Contributor

levino commented Jul 17, 2019

Nice. But not relevant for this PR. Please release, @NVIDIA. Just release it as 1.0.1 if need be.

@nivida
Copy link
Contributor Author

nivida commented Jul 22, 2019

@levino I‘m in contact with npm. We can‘t release 1.0 as 1.0.1 because 1.0.9 and 1.1.0 got published as well. I will ping them today again to get this process one step further. If npm isn’t answering or will reject the request will I release it as 1.2.0.

@levino
Copy link
Contributor

levino commented Jul 22, 2019

There is no problem in releasing it as 1.1.1 or 1.2.0. Please stop delaying this. You can release it immediately. Just do it.

@michaelsbradleyjr
Copy link
Contributor

michaelsbradleyjr commented Jul 22, 2019

If npm may make it possible for us to have 1.0.0 then I'd say it's worth waiting one more day. If by the end of today it's not possible to publish as 1.0.0 then 1.2.0 it is.

@levino
Copy link
Contributor

levino commented Jul 22, 2019

Why? Because you like "nice numbers"? It does not matter that the version string esthetically pleases your eyes. It matters that it is logically correct and people can f***ing use a reliable and functioning piece of software. Will you also oppose the release of version 1.0.13 because the thirteen is bad luck?

@levino
Copy link
Contributor

levino commented Jul 22, 2019

This "let's wait for npm" shit show has been going on for nine days already.

@levino
Copy link
Contributor

levino commented Jul 22, 2019

Also there are reasons why this is so hard. It is against all best practices and there is absolutely no need to unpublish at all. There is not one single rational reason to not just publish it as 1.2.0.

@Gudahtt
Copy link

Gudahtt commented Jul 22, 2019

While I don't think @levino 's disrespectful tone is helpful, I do agree that unpublishing seems like a very bad idea. Any projects using the existing 1.0 or 1.1 release would be in for an unpleasant surprise.

Releasing this as 1.2.0 does seem to have a lot of benefits and no downsides.

@levino
Copy link
Contributor

levino commented Jul 22, 2019

@Gudahtt Maybe it is not helpful, you are right. But then nobody here seems to be interested in making things work, so it cant harm either.

@wbt
Copy link
Contributor

wbt commented Jul 22, 2019

Why? Because you like "nice numbers"?

@levino This project has been obsessed with a "1.0.0" release for a long while now. As discussed here, this decision

seems like a somewhat arbitrary marketing-driven versioning decision and probably a bit of a lie on the "stable release" language which seems more likely meant to satisfy some VC or manager or PR purpose than to necessarily have the appropriate technical backing. (To me, changing to 1.0.0 could just be used to signify a breaking change; it doesn't have to be conflated with completion of a major milestone or even stability in the release, as seems to be conflated in Web3. Hyperledger Fabric put their first LTS release at whatever version number they were at when they thought they were ready, specifically 1.4).

Neither @Gudahtt nor I can see those downsides to choosing a higher version number because of the lack of transparency, but there are pretty strong signals that such downsides exist at least in @nivida's view. I don't think it's just aesthetics (and I don't think there's really a need for profanity either). @nivida has consistently refused to share exactly what the specific reasons behind the decisions (like the super strong emphasis on 1.0.0) are, but they are clearly strong ones whatever they may be. We will probably never really know, although disclosure would be much more consistent with open-source and the decentralization ethos this package is theoretically supposed to support. I suspect the actual reason is just so much less consistent with that ethos that it would seem embarrassing to share the real reasons, and that's why there's such a strong desire to not explain the strong motivators behind going specifically to a "1.0.0 'stable' release."

The page you (@levino) linked to warns that unpublish "is generally considered bad behavior." Abandoning semantic versioning, putting out major breaking changes with number updates more granular than patch, keeping decision-making behind closed doors without even explaining reasons for conclusions, and rejecting good-faith contributions on the grounds that they weren't written by core contributors who prefer their own hypothetical versions, are too. That apparently hasn't made a bit of difference; I doubt the linked-to warning will either. I do not think the assertion that "nobody here seems to be interested in making things work" is accurate though, or even close.

I'm not convinced this project is ready for a "stable release" label/claim. I am in favor of steps back toward semantic versioning and having some base version that can be patched/modified to resolve outstanding issues alongside appropriate bumps in the semver number depending on what gets changed, as things get changed, without burying @nivida or anybody else under a mountain of outstanding action items that all have to get changed at once before any new useful developments can be released. In the absence of being able to actually take those steps, could we please at least find out more about the real reasons why not?

@nivida
Copy link
Contributor Author

nivida commented Jul 22, 2019

@wbt

Neither @Gudahtt nor I can see those downsides to choosing a higher version number because of the lack of transparency, but there are pretty strong signals that such downsides exist at least in @nivida's view. I don't think it's just aesthetics (and I don't think there's really a need for profanity either). @nivida has consistently refused to share exactly what the specific reasons behind the decisions (like the super strong emphasis on 1.0.0) are, but they are clearly strong ones whatever they may be. We will probably never really know, although disclosure would be much more consistent with open-source and the decentralization ethos this package is theoretically supposed to support. I suspect the actual reason is just so much less consistent with that ethos that it would seem embarrassing to share the real reasons, and that's why there's such a strong desire to not explain the strong motivators behind going specifically to a "1.0.0 'stable' release."

We discussed the situation and other topics in the Ethereum Javascript Community (EJC) discord. The 1.2.0 release was seen as a worst-case solution. The worst-case solution wasn't taken because npm was answering on my support ticket and didn't reject the request as initially thought and "discussed" in the EJC. I've pinged them today and didn't get an answer until now and will do the release of 1.0 as 1.2.0 tomorrow (CEST).

The page you (@levino) linked to warns that unpublish "is generally considered bad behavior." Abandoning semantic versioning, putting out major breaking changes with number updates more granular than patch, keeping decision-making behind closed doors without even explaining reasons for conclusions, and rejecting good-faith contributions on the grounds that they weren't written by core contributors who prefer their own hypothetical versions, are too. That apparently hasn't made a bit of difference; I doubt the linked-to warning will either.

I'm maintaining this project currently for one year and the involvement of the community in decisions was always asked. I've asked for feedback in the personal blog post and the release announcement of the new architecture I've published and pull requests are welcomed and getting merged if the changes do have the quality I'm expecting. I've started to write down the first draft of a final 2.0 architecture here and hope many of you will be involved.

I'm not convinced this project is ready for a "stable release" label/claim. I am in favor of steps back toward semantic versioning and having some base version that can be patched/modified to resolve outstanding issues alongside appropriate bumps in the semver number depending on what gets changed, as things get changed, without burying @nivida or anybody else under a mountain of outstanding action items that all have to get changed at once before any new useful developments can be released.

The missing announcement will explain this closer and any further or required questions can be asked in the publicly accessible EJC discord or with an issue here on GitHub.

@wbt
Copy link
Contributor

wbt commented Jul 22, 2019

We discussed the situation and other topics in the Ethereum Javascript Community (EJC) discord. The 1.2.0 release was seen as a worst-case solution.

Thanks for posting the Discord link. I've just read through the complete history of the #web3js channel there. I found your messages saying "I would have to release 1.2 beacuse 1.1 is also existing" and "... [sic] and releasing of a 1.2.0 as initial stable release isn't something I will do" as well as "fuck off... I will release it as 1.2.0 because npm will probably anyways reject the request." There is no other characterization of this as a "worst-case solution" nor any reasons given why it's bad. It's just something you "won't do" for reasons that you consistently refuse to explain. WHY do you think it's a "worst-case" solution? What are the hidden downsides that we aren't seeing?

I'm maintaining this project currently for one year and the involvement of the community in decisions was always asked. I've asked for feedback in the personal blog post [no link] and the release announcement [no link] of the new architecture I've published and pull requests are welcomed and getting merged if the changes do have the quality I'm expecting. I've started to write down the first draft of a final 2.0 architecture here [no link] and hope many of you will be involved.

I maintain this request:

that any discussions in a more rapid synchronous channel (like an in-person meeting, Discord, or teleconference) should be summarized on the archival listserv (now, it would be GitHub Issues), including who was there, what decisions were made, the reasons for them, and reasons for an alternative path that were considered but overridden. I would like to see more of that happening in these projects.

...please!

I'm not convinced this project is ready for a "stable release" label/claim....

The missing announcement will explain this closer and any further or required questions can be asked in the publicly accessible EJC discord or with an issue here on GitHub.

Here is that Issue.

@web3 web3 locked as too heated and limited conversation to collaborators Jul 22, 2019
@nivida
Copy link
Contributor Author

nivida commented Jul 22, 2019

@wbt Truffle and OpenZeppelin was the main actor to release beta.37 as 1.0 stable version of web3.js because it's used widely over the community. We were discussing this for a long time and I was first against this decision and would have liked to follow the path with the new architecture I've written since I've started here but this isn't possible because some projects were relying on the internal API and architecture of web3.js. Splitting this up between a 1.0 and 2.0 version of web3.js will give us the possibility to not break already existing DApp's. The new architecture in 2.0 does provide the same public API as we had before just with new core modules and some additional features but the real benefit of 2.0 is that we are able to use the latest features of JavaScript. Version 1.0 and also 2.0 will be maintained and improved by me and the community which wants to be involved.

I've locked the conversation of this PR because version 1.2.0 gets released as the initial non-beta version of the 1.0 architecture of web3.js tomorrow. Further discussion can be held over GitHub or the mentioned discord channel above.

@nivida nivida merged commit 54feed8 into 1.x Jul 23, 2019
@nivida nivida deleted the release/1.0 branch August 2, 2019 20:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.x 1.0 related issues Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.