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 Process Improvements #3358

Closed
nivida opened this issue Feb 2, 2020 · 11 comments · Fixed by #3460
Closed

Release Process Improvements #3358

nivida opened this issue Feb 2, 2020 · 11 comments · Fixed by #3460
Labels
Enhancement Includes improvements or optimizations Release

Comments

@nivida
Copy link
Contributor

nivida commented Feb 2, 2020

Description

With the last security patch release did we all noticed that the existing release guidelines aren't prepared for such an emergency case. The goal of this issue would be to discuss this in-depth and to create a related PR with the required improvements of the release guideline. Those rules should also be defined for all EF JS Team projects (A related issue does already exist here).

By side of the required improvements of the guideline have we to overthink the whole process to provide the "non-involved" contributors a better overview during the release process and to automate as much as we can to prevent us from human failure. This could, for example, be achieved by introducing conventional commits and automating the release through semantic-release.

Related issues:

Required Release Guideline Changes

  • The rules have to distinct between patch, minor, and major version releases.
  • Special rules for security patches have to be defined.
  • Defining of clear rules for the changeset a release PR can have.
    • This is probably something that only does affect the emergency case.
  • A list of tasks we have to do during the release and after it got successfully published
  • Defining of different tracks we do have on npm

Feel free to drop a comment here with your ideas and what you think about the proposed improvements from above. @holgerd77 @alcuadrado @michaelsbradleyjr @iurimatias @gnidan @cgewecke @joshstevens19 @JosefJ

@nivida nivida added Enhancement Includes improvements or optimizations Discussion Release labels Feb 2, 2020
@nivida nivida changed the title Release Process Improvments Release Process Improvements Feb 2, 2020
@nivida
Copy link
Contributor Author

nivida commented Feb 2, 2020

All of those mentioned ideas and improvements should prevent us from having long discussions on a release PR which is frustrating for the releasing contributor and the involved reviewers. :)

Edit:
This is probably a good introduction to read about the problem we have and the existing solutions with the related philosophy behind. Didn't read it completely until now.

Edit2:
I've gone through the shape up book and it didn't give me the inputs I wished it would. I will now read how other bigger projects as Angular, VueJS, React, TypeScript, and NodeJS have their guidelines and will follow those already well-defined rules.

@holgerd77
Copy link
Collaborator

One general note: I think to some greater extend the CONTRIBUTIONS guidelines for PRs and releases you guys set up already worked pretty well and show already some effect on improving the release quality.

Since the guidelines are pretty new there are likely things which are not well defined yet and need to be added, generally this will be something which will likely continue to evolve over time. This also doesn't completely eliminates the need and (mostly 😜) benefit to have some last discussions around releases, it will likely be reoccurring that there will be some last specifics to settle out, which is at the end a good thing. This is what this one week RC period is actually is meant for, to give the opportunity to collect voices from within the team as well as the community.

For the case we had, we might want to add some explicit notes, of course we can also improve on other topics (some distinction on major/minor/patch release as suggested by @nivida likely also makes sense). There was some consensus among the reviewers that there just shouldn't be additional changes in a emergency release going beyond the scope of what is to be addressed on the specific emergency case. This actually complies really well with the existing defined release rules, so to have all the changes (respectively: releases) under a one week review period.

If we include unrelated changes/PRs in an emergency release, these changes are unnecessarily taken out of the one week review process. This adds an unnecessary release risk and should therefore be avoided. Is this a fair summing up?

@nivida
Copy link
Contributor Author

nivida commented Feb 4, 2020

@holgerd77
I and @cgewecke do currently work out a release and review guideline and will present it here to the community as soon as it is reviewed internally and ready. Those releases and review guidelines are inspired by the biggest JS projects we know. (informed you about this on gitter)

The mentioned release automation will be split up in clear tasks and tracked with an overall issue as epic.

@nivida nivida removed the Discussion label Feb 4, 2020
@nivida
Copy link
Contributor Author

nivida commented Feb 4, 2020

If we include unrelated changes/PRs in an emergency release, these changes are unnecessarily taken out of the one week review process. This adds an unnecessary release risk and should therefore be avoided. Is this a fair summing up?

I was the meaning in the release PR that because those implemented changes are on a patch level and definitely non-breaking would it have been possible to release the security-related changes as a patch release. The test suite we have does cover our exposed interface. The violation of the release guideline we had was that I thought we can release such a patch release also without an RC version. This because it doesn't really make sense for me to have RC releases on an increase on the non-breaking patch level as defined in semver and especially not in an emergency case.
This was the point I've noticed that we do have to improve the release process to achieve the professionality we all would like to have.

@cgewecke
Copy link
Collaborator

cgewecke commented Feb 5, 2020

If we include unrelated changes/PRs in an emergency release, these changes are unnecessarily taken out of the one week review process. This adds an unnecessary release risk and should therefore be avoided. Is this a fair summing up?

This seems correct to me.

For further context, the .rc for all releases is a recommendation of #3070 and I think one goal there is to introduce a safety margin above and beyond traditional semver constraints because that seems appropriate for 1.x given its complex publication history, with a long beta cycle etc.

@nivida
Copy link
Contributor Author

nivida commented Feb 5, 2020

For further context, the .rc for all releases is a recommendation of #3070 and I think one goal there is to introduce a safety margin above and beyond traditional semver constraints because that seems appropriate for 1.x given its complex publication history, with a long beta cycle etc.

Thanks for your input Chris! :)

I'm not sure if I do read it wrongly but for me where we talking there about "an RC release" and not RC releases. That’s why I first thought to release those improvements in one for example minor version where an RC version sometimes (especially in this case) does make sense. But because we decided to release those improvements on several patch level increases did we had to publish RC versions for those patches :)

I think we can release an RC version on the next release as well because of the newly added features and bigger improvements it will have (PRI, ENS). But we should as soon as possible be align with semver (patch === 100% non-breaking).

that seems appropriate for 1.x given its complex publication history, with a long beta cycle etc.

Can you elaborate it closer? This is for me a vague heart statement without facts:)

@nivida
Copy link
Contributor Author

nivida commented Feb 5, 2020

@cgewecke Addition:
With the wished conventional commits and automated release process through semantic-releases from the community will we have to change this anyways to be strictly aligned with the given standards the community would like to see.

@cgewecke
Copy link
Collaborator

cgewecke commented Feb 6, 2020

that seems appropriate for 1.x given its complex publication history, with a long beta cycle etc.

Can you elaborate it closer? This is for me a vague heart statement without facts:)

Fair enough. I think if you look at #3070 as a detailed survey of 180 issues over 18 months you get a concrete sense of the project's complexity and how challenging it's been to develop without accidentally introducing problems. Have uncommented one of the sections there that was omitted for clarity/brevity so the complete picture is more visible.

Slowly accumulating releases which "just work" is likely the most important fact (or test) from a users standpoint. And it will take time to accomplish that.

@nivida
Copy link
Contributor Author

nivida commented Feb 6, 2020

@cgewecke Thanks for sharing your standpoint!

Fair enough. I think if you look at #3070 as a detailed survey of 180 issues over 18 months you get a concrete sense of the project's complexity and how challenging it's been to develop without accidentally introducing problems.

The out-commented list from issue #3070 is as the title is saying only 2.0 related. I've rewritten the entire library within 6 months (not 18; includes bug fixing) and was on the same stage of stability as we are now. You are right I've introduced with the complete re-write breaking changes but those changes could have been listed with ease and aren't that much. Additionally, do I think can we not compare a beta prelease state (break; improve; fix) with the production version (LTS state) range we are now.

Have uncommented one of the sections there that was omitted for clarity/brevity so the complete picture is more visible.

I don't think this will bring up more clarity and I will out-comment it again after the discussion here is finished:)

Slowly accumulating releases which "just work" is likely the most important fact (or test) from a users standpoint. And it will take time to accomplish that.

I don't think this will take extra time to accomplish it. This because we already achieved it. We didn't introduce any breaking changes for our depending projects since the production version got published.

with a long beta cycle

Yes, there was a long beta cycle but mostly because the former maintainer didn't have the time to work as much as he probably would have liked to work on this project. The recommendation I got from him when he was over giving me the project was to stay longer in the beta state and fix anything which has to get fixed.

I'm still asking my self why exactly so many devs were building up entire projects on a beta pre-release which are for example used by Microsoft. I've learned that beta versions can be used for prototypes but for real-world products should a production-ready or at least marked as non-breaking be used e.g.: 0.20.x.

Edit:
Checked that closer. 9 months took the refactoring and fixing of bugs to achieve the same stability (includes provider improvements). The source of the refactoring is #2000. I've also added a lot of backward compatibility improvements in the architecture of the 2.x branch. By side of just having the same stability did the code quality improved by a minimum of 10x, I've cleaned up and labeled the entire issue list (500+), and I've done all the other stuff which does run by side of coding as a maintainer of such an OSS project.

@nivida
Copy link
Contributor Author

nivida commented Feb 6, 2020

My conclusion out of your helpful view you shared and the experience I have with the functionality and architectures behind this project is still that our QA and the project management related knowledge base we have does guarantee us to be able to be aligned with semver as mentioned above from me. :)

But I think it is worth to get a third opinion from a non-involved experienced member from the EF-JS-Team as for example @holgerd77 :)

@holgerd77
Copy link
Collaborator

I think we are mixing very much too many things up here, and to discuss here how to proceed on 2.0 and when (and how) to get aligned with semver again overloads the discussion here and will bring us too much off the track. These things are better (and already are, e.g. in #3228) to be discussed in separated issues.

Issue here is about the release process itself respectively the guidelines around it. @nivida as we agreed in our call from yesterday, the situation on release quality is already better than eventually perceived from the outside. For this to trickle through it will nevertheless take more time, view here is still very much shaped from the (understandable) problems during 2019.

So I think we should at least temporarily keep the habit of doing RCs on just every release, this will include patch release as well (I don't think doing RCs conflicts with semver in any regard, or am I missing something here?). I think the overhead is limited here, and even if the perception of the community on the quality of the library already improved significantly (I would very much assume so) this allows us to build up additional trust which just won't be a bad thing in any regard.

One key important aspect here is also: the non-breaking nature of releases is not always clear. As you (@nivida) stated (I think also in our call?) people are using the Web3.js library in very much unintended und surprising ways. So there is a non-trivial chance that changes strictly not being breaking on the API level might cause unexpected problems for people. That is also what an RC version is good for and very helpful, there is just a chance for community members to intervene when there is just not a chance for a maintainer to foresee what changes will eventually cause side effects.

One thing I would suggest we can discuss here though: a RC waiting period of 7 days for patch releases might actually be a bit long to keep on with some fluffy development process. So eventually we want to reduce the period for patch releases, I would suggest 3 full work days here (so that an RC published throughout a Monday can e.g. be published on a Thursday as the final version).

What do you guys think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Includes improvements or optimizations Release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants