Skip to content

Latest commit

 

History

History
466 lines (315 loc) · 17.5 KB

COLLABORATOR_GUIDE.md

File metadata and controls

466 lines (315 loc) · 17.5 KB
Raw

Collaborator Guide

Table of Contents

Issues and Pull Requests

Full courtesy should always be shown in video.js projects.

Collaborators may manage issues they feel qualified to handle, being mindful of our guidelines.

Any issue and PR can be closed if they are not relevant, when in doubt leave it open for more discussion. Issues can always be re-opened if new information is made available.

If issues or PRs are very short and don't contain much information, ask for more by linking to the issue or PR template. There is also a response guide if you're unsure.

Labels

There are labels that are useful to include on issues and PRs. A few of them are defined below:

Label Issue or PR Description
confirmed Issue and PR Issue: marks as reproducible. PR: marks as ready to be merged
5.x PR Marks as a change to the 5.x branch only
bug Issue Marks as a confirmed bug
good first issue Issue Marks as a good bug or enhancement for first time contributors to Video.js
first-timers-only Issue Marks as a good bug or enhancement to be done by a newcomer to open source
minor, patch, major PR Marks PR with the expected semver classification of the change
needs: LGTM PR Marks PR to be reviewed by a collaborator
needs: more info Issue Marks as needing more information from the issue reporter
needs: reduced test case Issue Marks as needing a reduced test case from the issue reporter

Accepting changes

Any code change in video.js should be happening through Pull Requests on GitHub. This includes core committers.

Before a PR is merged, it must be reviewed by at least two core committers, at least one if it comes from a core committer.

Feel free to @-mention a particular core committer if you know they are experts in the area that is being changed.

If you are unsure about the modification and cannot take responsibility for it, defer to another core committer.

Before merging the change, it should be left open for other core committers to comment on. At least 24 hours during a weekday, and the 48 hours on a weekend. Trivial changes or bug fixes that have been reviewed by multiple committers may be merged without delay.

For non-breaking changes, if there is no disagreeming between the collaborators, the PR may be landed assuming it was reviewed. If there is still disagreement, it may need to be escalated to the TSC.

Bug fixes require a test case that fails beforehand and succeeds after. All code changes should contain tests and pass on the CI.

Involving the TSC

A change or issue can be elevated to the TSC by assing the tsc-agent label. This should be done in the following scenarios:

  • There will be a major impact on the codebase or project
  • The change is inherently controversial
  • No agreement was reached between collaborators participating in the discussion

The TSC will be the final arbiter when required.

Landing a PR

Landing a PR is fairly easy given that we can use the GitHub UI for it.

When using the big green button on GitHub, make sure the "squash and merge" is selected -- it should be the only allowed option. If a PR has two features in it and should be merged as two separate commits, either ask the contributor to break it up into two, or follow the manual steps.

The commit message should follow our conventional changelog conventions. They are based on the angularjs changelog conventions. The changelog is then generated from these commit messages on release.

The first line of the commit message -- the header and first text box on GitHub -- should be prefixed with a type and optional scope followed by a short description of the commit. The type is required. Two common ones are fix and feat for bug fixes and new features. Scope is optional and can be anything.

The body should contain extra information, potentially copied from the original comment of the PR.

The footer should contain things like whether this is a breaking change or what issues were fixed by this PR.

Here's an example:

fix(html5): a regression with html5 tech

This is where you'd explain what the regression is.

Fixes #123

Landing a PR manually

Optional: ensure you're not in a weird rebase or merge state:

git am --abort
git rebase --abort

Checkout and update the master branch:

git checkout master
git remote update
git rebase upstream/master

Check out the PR:

git fetch upstream pull/{{PR Number}}/head:{{name of branch}}
git checkout -t {{name of branch}}

For example:

git fetch upstream pull/123/head:gkatsev-html5-fix
git checkout -t gkatsev-html5-fix

Optional: If necessary, rebase against master. If you have multiple features in the PR, landing a PR manually with several changes

git rebase master

Fix up any issues that arise from the rebase, change back to the master branch and squash merge:

git checkout master
git merge --squash --no-commit gkatsev-html5-fix

The --no-commit tells git not to make a commit on your behalf. It does stage everything for you, so, you can instead it:

git diff --cached

Now get the author from the original commit:

git log -n 1 --pretty=short gkatsev-html5-fix

Which shows:

  commit 433c58224f5be34480c8e067ca6c5406ba1c1e9c
  Author: Gary Katsevman <git@gkatsev.com>

      Update TOC

Now you can commit the change the change with the author, following our commit guidelines

git commit --author "Gary Katsevman <git@gkatsev.com>"

Now that it's committed, push to master

git push upstream master

Congratulate yourself for a job well done and the contributor for having his change landed in master.

Landing a PR manually with several changes

Follow the same steps as before but when you rebase against master, you want to do an interactive rebase and then squash the changes into just a few commits.

git rebase -i master

This will give you an output like the following:

pick b4dc15d Update CONTRIBUTING.md with latest info
pick 8592149 Add Dev certificate of origin
pick 259dee6 Add grunt and doctoc npm scripts
pick f12af12 Add conventional-changelog-videojs link
pick ae4613a Update node's CONTRIBUTING.md url
pick 433c582 Update TOC

# Rebase f599ef4..433c582 onto f599ef4 (6 command(s))
#
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
# f, fixup = like "squash", but discard this commit's log message
# x, exec = run command (the rest of the line) using shell
# d, drop = remove commit
#
# These lines can be re-ordered; they are executed from top to bottom.
#
# If you remove a line here THAT COMMIT WILL BE LOST.
#
# However, if you remove everything, the rebase will be aborted.
#
# Note that empty commits are commented out

Replace pick to fixup or edit depending on how you want the output to look. You can also re-order the commits, if necessary.

fixup will squash the commit it's infront of up into the commit above it

edit will allow you to edit the commit message before continuing

edit b4dc15d Update CONTRIBUTING.md with latest info
fixup 8592149 Add Dev certificate of origin
fixup f12af12 Add conventional-changelog-videojs link
fixup ae4613a Update node's CONTRIBUTING.md url
fixup 433c582 Update TOC
edit 259dee6 Add grunt and doctoc npm scripts

When you get to the edit commits, git will give more information, but you'd want to run ammend the current commit while following our commit guidelines

git commit --amend

After going through and making the commits you want, you want to change back to master and then rebase the branch onto master so we get a clean history

git rebase gkatsev-html5-fix

This will put our two commits into master:

b4dc15d chore(contributing.md): Update CONTRIBUTING.md with latest info <Gary Katsevman>
259dee6 chore(package.json): Add grunt and doctoc npm scripts <Gary Katsevman>
9e20386 v5.12.6 <Gary Katsevman>

Now you're ready to push to master as in the normal instructions.

I just made a mistake

While git allows you to update the remote branch with a force push (git push -f). This is generally frowned upon since you're rewriting public history. However, if you just pushed the change and it's been less than 10 minutes since you've done with, you may force push to update the commit, assuming no one else has already pushed after you.

I accidentally pushed a broken commit or incorrect commit to master

Assuming no more than 10 minutes have passed, you may force-push to update or remove the commit. If someone else has already pushed to master or 10 minutes have passed, you should instead use the revert command (git revert) to revert the commit and then commit the proper change, or just fix it forward with a followup commit that fixes things.

I lost changes

Assuming that the changes were committed, even if you lost the commit in your current history does not mean that it is lost. In a lot of cases you can still recover it from the PR branch or if all else fails look at git's reflog.

I accidentally committed a broken change to master

This is a great time to discover that something is broken. Because it hasn't been pushed to GitHub yet, it's very easy to reset the change as if nothing has happened and try again.

To do so, just reset the branch against master.

git reset --hard upstream/master

video.js releases

Releasing video.js is partially automated through various scripts. To do a release, you need a couple of things: npm access, GitHub personal access token.

Releases in video.js are done on npm and GitHub and eventually posted on the CDN. These are the instructions for the npm/GitHub releases.

When we do a release, we release it as a next tag on npm first and then at least a week later, we promote this release to latest on npm.

Getting dependencies

npm access

To see who currently has access run this:

npm owner ls video.js

If you are a core committer, you can request access to npm from one of the current owners. Access is managed via an npm organization for Video.js.

GitHub personal access token

This is used to make a GitHub release on videojs. You can get a token from the personal access tokens page.

After generating one, make sure to keep it safe because GitHub will not show the token for you again. A good place to save it is Lastpass Secure Notes.

Deciding what type of version release

Since we follow the conventional changelog conventions, all commits are prepended with a type, most commonly feat and fix. If all the commits are fix or other types such as test or chore, then the release will be a patch release. If there's even one feat, the release will be a minor release. If any commit has a BREAKING CHANGE footer, then the release will be a major release. Most common releases will be either patch or minor.

Doing a release

It is also recommended you have a clean clone of Video.js for each release line you want to release. That means having a folder for master/v6 and one for 5.x. This is because 5.x and 6.x have different versions expecations for release process and have different dependencies. Plus, during development you could end up with a dirty repo, so, it just usually easier if you have a clean release repo.

# for v6
git clone git@github.com:videojs/video.js.git videojs-6-release
# for v5
git clone git@github.com:videojs/video.js.git videojs-5-release

Video.js 6

Make sure go to the master branch and grab the latest updates.

git checkout master
git pull origin master

At this point, you should run npm install because dependencies may have changed.

Then, it's mostly a standard npm package release process with running npm version, followed by an npm publish.

npm version {major|minor|patch}

Depending on the commits that have been merged, you can choose from major, minor, or patch as the versioning values. See deciding what type of version release section.

Optionally, you can run git show now to verify that the version update and CHANGELOG automation worked as expected.

Afterwards, you want to push the commit and the tag to the repo. It's necessary to do this before running npm publish because our GitHub release automation relies on the commit being available on GitHub.

git push --tags origin master

Finally, run npm publish with an appropriate tag. Don't forget to supply your token.

VJS_GITHUB_USER=gkatsev VJS_GITHUB_TOKEN=my-personal-access-token npm publish --tag next

After it's done, verify that the GitHub release has the correct changelog output. This is to make sure that the CHANGELOG didn't get garbled and isn't missing pieces.

If the GitHub release did not work correctly, such as if the GitHub token was not provided, you can run it manually:

VJS_GITHUB_USER=gkatsev VJS_GITHUB_TOKEN=123 node build/gh-release.js --prelease

Video.js 5

Make sure to go to the 5.x branch and grab the latest updates.

git checkout 5.x
git pull origin 5.x

Note: you probably need to delete v6 tags due to the way that the our CHANGELOG lib works.

You can run this to delete them:

git tag | grep '^v6' | xargs git tag -d

This will find all tags that start with ^v6 and delete them.

At this point, you should run npm install because dependencies may have changed.

Then, we have a script that automates most of the steps for publishing. It's a little trickier than publishing v6.

Edit git-semver-tags

You'll need to edit git-semver-tags to support our usage of tags that are not part of the branch. In the file node_modules/conventional-changelog-cli/node_modules/conventional-changelog/node_modules/conventional-changelog-core/node_modules/git-semver-tags/index.js, edit the line that says sets the cmd to be:

var cmd = 'git log --all --date-order --decorate --no-color';

And now for the release

After getting rid of the tags and getting the latest updates, you can just run the release script:

VJS_GITHUB_USER=gkatsev VJS_GITHUB_TOKEN=123 ./build/bin/release-next.sh

It will prompt you for a version change type, so, input patch or minor or major. See deciding what type of version release section.

When it's done building everything, it'll show you the commit that's made via the default pager (i.e., less). At this point you can verify that things look normal rather than, for example, missing all the CSS.

After exiting the pager, it'll make sure you want to continue with publishing.

It will automatically release it as a next-5 tag on npm.

Then push the local changes up:

git push --tags origin 5.x

Also, you'll need to copy the CHANGELOG for this version and manually edit the GitHub release to include it. The current release's CHANGELOG could be copied from the raw CHANGELOG.md file (or locally from the markdown file) and then pasted into the correct GitHub release.

Deploy as a patch to the CDN

Follow the steps on the CDN repo for the CDN release process. If it's a next or next-5 release, only publish the patch version to the CDN.

When the version gets promoted to latest or latest-5, the corresponding minor or latest version should be published to the CDN.

Announcement

An announcement should automatically make it's way to #announcements channel on slack, it uses IFTTT and might take a while.

You can also post it to twitter or ask someone (like @gkatsev) to post on your behalf.

If it's a large enough release, consider writing a blog post as well.

Doc credit

This collaborator guide was heavily inspired by node.js's guide