Skip to content

Fix release instructions #158

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

Merged
merged 4 commits into from
Jul 26, 2017
Merged

Fix release instructions #158

merged 4 commits into from
Jul 26, 2017

Conversation

darrnshn
Copy link
Contributor

@darrnshn darrnshn commented Jul 21, 2017

This patch updates the release instructions in CONTRIBUTING.md:

  • Add instructions to get fresh clone of repo.
  • Added --ignore-unmatch to git rm .gitignore because .gitignore may not exist.
  • Clarify where to push things.
  • Replaced git push HEAD:refs/heads/master with git push origin master because the former simply did not work.
  • Add more detailed instructions for drafting the release on GitHub.

Note that this diff contains an irrelevant commit Fix missing web-animations.min.js issue. This will disappear once that commit is pushed to dev by @ewilligers .

Copy link
Contributor

@alancutter alancutter left a comment

Choose a reason for hiding this comment

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

Yay!

git commit -m 'Add build artifacts from '`cat .git/refs/heads/dev`
git push HEAD:refs/heads/master
git push origin master
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a step to verify that git remote -v shows that origin points to github.com/web-animations/web-animations-js.

Copy link
Contributor

Choose a reason for hiding this comment

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

Steps "Add versioned release notes to History.md" and "Commit the above changes" should probably also be using origin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm what happens if their git remote -v doesn't show the right value? If it's a fork, I'm guessing it'll point to their own forked repo?

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case the least invasive way to resolve it is to make a new clone of the central repo and work in that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, so maybe for releases, we use a clone, but for code changes, use fork? I'll add a step in the beginning for cloning the repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure that works. If the release process doesn't assume the existence of a repository that gives it more guarantees on the repo's state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, uploaded new commits, PTAL

History.md Outdated
@@ -1,3 +1,7 @@
### 2.3.1 - *July 20 2017*

* Fix [https://github.com/web-animations/web-animations-js/issues/157](missing web-animations.min.js issue)
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is unrelated and should be kept separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it looks like I didn't actually push to origin dev (not sure if I was meant to).
@ewilligers will be pushing that commit to origin dev soon so this diff should disappear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmm, the instructions should probably say to push to origin/dev to avoid this problem in future.

package.json Outdated
@@ -5,7 +5,7 @@
"type": "git",
"url": "https://github.com/web-animations/web-animations-js.git"
},
"version": "2.3.0",
"version": "2.3.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is unrelated and should be kept separate.

@darrnshn darrnshn merged commit 910a48d into dev Jul 26, 2017
@darrnshn darrnshn deleted the fix-release-instructions branch July 26, 2017 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants