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

chore: add prerelease script #876

Merged
merged 10 commits into from
Aug 1, 2018
Merged

chore: add prerelease script #876

merged 10 commits into from
Aug 1, 2018

Conversation

endiliey
Copy link
Contributor

@endiliey endiliey commented Jul 30, 2018

Motivation

Simplify maintainer process before releasing new version

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Click this asciinema preview
https://asciinema.org/a/hZ7NNJPcgtdvzm5tMLLfHflMD
asciicast

1

2

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jul 30, 2018
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jul 30, 2018

Deploy preview for docusaurus-preview ready!

Built with commit e87969a

https://deploy-preview-876--docusaurus-preview.netlify.com

Copy link
Contributor

@JoelMarcey JoelMarcey left a comment

Choose a reason for hiding this comment

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

This is great!

A couple of comments:

  1. Does this presume that we will still update the .lock files automatically? -- there is an outstanding question whether we should have .lock files at all.

  2. We could extend this to start the Changelog updates for us too, maybe? 😄

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

Looks good. Just some questions regarding its usage.


# Steps

1. Ensure that `origin` remote is your Docusaurus fork. Example
Copy link
Contributor

Choose a reason for hiding this comment

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

Before executing any of these steps, we will have to make the changes in the CHANGELOG, etc, is that right? If so, we should add it before this step.

Copy link
Contributor Author

@endiliey endiliey Jul 31, 2018

Choose a reason for hiding this comment

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

Yes we had to make changes in CHANGELOG, etc as well.

However, for CHANGELOG I propose that we now start adding changelog everytime we merge PR just like how jest do it. This will make it easier for our process.

Example:
A recently closed PR in jest https://github.com/facebook/jest/pull/6776/files has a changelog commit as well

Then we will always have a changelog for master version
screenshot_20180731-070432

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not opposed to trying that but feel that it's hard for Docusaurus PR submitters to write the CHANGELOG. We'll need to create a template for the next version and pending PRs would always have to take note of which section of CHANGELOG they're updating. Might be a source of frequent merge conflicts. @JoelMarcey thoughts?

Copy link
Contributor Author

@endiliey endiliey Jul 31, 2018

Choose a reason for hiding this comment

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

Alternatively, the person who merge the PR should update the changelog just right before merging it.

So PR submitter doesn't have to do it

fi
fi

read -p "Release $VERSION - are you sure? (y/n) " -n 1 -r
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 "Submit a PR for a bump in $VERSION version - Are you sure ..." is clearer.

NEW_VERSION=$(node -p "require('./package.json').version")

# create new branch
git checkout -b $NEW_VERSION master
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 we should create new branch before bumping version just to be safe.

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 did this because I wanted to get the new version (after bump) as the branch name. How about naming it to prerelease ? We can also delete the existing local prerelease branch as well before creating new branch

Copy link
Contributor

Choose a reason for hiding this comment

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

Mm gotcha. Let's leave this as it is. On second thought it's fine because we're not committing yet.

git checkout -b $NEW_VERSION master

# cut docusaurus docs version
cd website && yarn run version $NEW_VERSION
Copy link
Contributor

Choose a reason for hiding this comment

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

Omg we should probably rename this command within website/package.json. It's easy to confuse with yarn's native version command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will docsversion do?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. Will need to update a few places as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually i wanted to rework all the CLI commands to docusaurus <command> instead of docusaurus-command but this might lead to breaking changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yes we should do that, but not now. I fully agree with you (wrote about that in v2 under API) also.

@endiliey
Copy link
Contributor Author

endiliey commented Aug 1, 2018

This might need re-review @JoelMarcey @yangshun
I think this is good. There is a room of improvement for the wording but shouldn't matter a lot since this script is only used by maintainer.

Note that this doesn't really update CHANGELOG.md or other files automatically. I have updated the instruction and the asciinema preview to show that you have to update CHANGELOG & other files.

@endiliey endiliey merged commit 3971f80 into facebook:master Aug 1, 2018
@endiliey endiliey deleted the prerelease branch August 1, 2018 17:10
@JoelMarcey
Copy link
Contributor

Awesome. I am looking forward to one of us trying this.

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

Successfully merging this pull request may close these issues.

5 participants