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

Introduce Review and Release Guidelines #3460

Merged
merged 11 commits into from
Apr 29, 2020
Merged

Conversation

ryanio
Copy link
Collaborator

@ryanio ryanio commented Apr 8, 2020

Description

This PR introduces REVIEW.md and RELEASE.md, two new documents that specify review and release guidelines and procedures, adapted from @nivida's documents Review_Guidelines.md and Release_Guidelines.md.

Closes #3358

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • My changes generate no new warnings.
  • I have updated the CHANGELOG.md file in the root folder.

@ryanio ryanio added the Documentation Relates to project wiki or documentation label Apr 8, 2020
@coveralls
Copy link

coveralls commented Apr 8, 2020

Coverage Status

Coverage remained the same at 88.596% when pulling ec23c67 on reviewAndReleaseGuidelines into 3684fc3 on 1.x.

@ryanio ryanio added the Review Needed Maintainer(s) need to review label Apr 8, 2020
Copy link
Collaborator

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

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

This looks great 🎉

A technical detail that might be helpful to document is the publishing command for both the rc and formal releases - like "how to tag the rc on npm" etc.

Should the 1 week waiting period be considered flexible for emergency patches or releases?

FWIW there's precedent for this in the 1.2.6 release (see #3351). The consensus view there was to make the smallest change necessary to address the emergency while waiving the rc process (meaning many existing additions to master were excluded).

That's pretty conservative and might not be appropriate for every emergency though.

RELEASE.md Outdated Show resolved Hide resolved
RELEASE.md Outdated Show resolved Hide resolved
@ryanio
Copy link
Collaborator Author

ryanio commented Apr 8, 2020

A technical detail that might be helpful to document is the publishing command for both the rc and formal releases - like "how to tag the rc on npm" etc.

@cgewecke thanks sounds good. Creating the rc track in npm is still on my TODO list.

re: publishing command, how does this look?
npm dist-tag add <package-name>@<version> [<tag>]

@cgewecke
Copy link
Collaborator

cgewecke commented Apr 8, 2020

@ryanio

npm dist-tag add @ []

Yes that LGTM ... do you have a view about whether lerna should be involved here at all? I think they have an npm tagging option as well.

@ryanio
Copy link
Collaborator Author

ryanio commented Apr 8, 2020

Yes that LGTM ... do you have a view about whether lerna should be involved here at all? I think they have an npm tagging option as well.

Seems reasonable, I'll have to investigate further. @nivida said lerna has been not-so-reliable in the past:

"mostly it works.. but there are a few times it doesn't"
-@nivida

so we can try it but we may need some backup options if the lerna command doesn't work as planned. I've commonly heard of others (@ricmoo, @alcuadrado) rolling their own lerna-esque solutions. I think if it fails partially through is where the headaches come in, since some packages would have been run and others not.

@ryanio ryanio force-pushed the reviewAndReleaseGuidelines branch 3 times, most recently from 4a42c73 to 039f7ea Compare April 9, 2020 01:34
@ryanio ryanio force-pushed the reviewAndReleaseGuidelines branch from 039f7ea to a5a768d Compare April 9, 2020 01:42
@holgerd77
Copy link
Collaborator

@ryanio Great stuff! 😄 👍

@cgewecke cgewecke self-requested a review April 9, 2020 15:13
cgewecke
cgewecke previously approved these changes Apr 9, 2020

### Patch

Since November 2019, Web3 has been following the `minor` rules for `patch` as well, to help establish a series of non-breaking releases.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could get changed to a normal process aligned with semver. I and Chris have done the big work already without any breaking change and plenty of new test cases :-)

| Release | Status | Initial Release | LTS Start | End-of-Life |
| :-----: | :-----------: | :-------------: | :-----------: | :---------: |
| 1.x | LTS | 24. Jul. 2017 | 23. Jul. 2019 | TBD |
| 2.x | Alpha Preview | 13. Jul. 2019 | TBD | TBD |
Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed the LTS and EOL of 2.x already with Embark, Travis, etc... If I'm correct did we said 2.0 stable release plus 12months == EOL 1.0


Only published PRs will be considered for review. Draft PRs will be considered in-progress and not yet ready for review.

## Rules
Copy link
Contributor

Choose a reason for hiding this comment

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

This was a list I wrote down quickly in some minutes.. I think this could get worked out to a real document.. idk :-)

btw..: Thanks Ryan for taking those drafts to the next step! 💪

@nivida
Copy link
Contributor

nivida commented Apr 15, 2020

overall and probably off-topic: My personal wish and the wish from the community was to have automated releases.. is this still an idea? or do we get at least a nightly version on npm?

@ryanio
Copy link
Collaborator Author

ryanio commented Apr 17, 2020

overall and probably off-topic: My personal wish and the wish from the community was to have automated releases.. is this still an idea?

I'm personally in favor of automated releases.

or do we get at least a nightly version on npm?

We can build a nightly version pretty easily now that we are running on GH Actions.

@ryanio ryanio force-pushed the reviewAndReleaseGuidelines branch 2 times, most recently from 7cf7344 to 5f0a6d1 Compare April 29, 2020 18:33
@ryanio ryanio force-pushed the reviewAndReleaseGuidelines branch from 5f0a6d1 to 871432a Compare April 29, 2020 18:35
@cgewecke cgewecke self-requested a review April 29, 2020 18:58
Copy link
Collaborator

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

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

🎉

@ryanio ryanio merged commit 3e10668 into 1.x Apr 29, 2020
@holgerd77 holgerd77 deleted the reviewAndReleaseGuidelines branch April 30, 2020 15:29
@ryanio ryanio mentioned this pull request May 8, 2020
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Relates to project wiki or documentation Review Needed Maintainer(s) need to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release Process Improvements
5 participants