-
Notifications
You must be signed in to change notification settings - Fork 0
Pull Request Advice
Remember to lint (yarn lint
) and test (yarn test
) your changes. Also keep an eye on Travis CI https://travis-ci.com/mezzode/cs4920-project/pull_requests so you can tell if something breaks the build.
Prefer small(ish) commits that can be explained in the commit message. This means that we will have a useful history to fall back on. If your commits are not informative, we'll have to squash them to keep the history clean(ish).
Make pull requests early and push updates to it so we can all see what's going on and give feedback. Prefix the title with [WIP]
and the WIP
label if it's not ready. When it is, remove them and add reviewers.
You should prefer smaller pull requests so features can be individually accepted or rejected. For example, develop your front and back ends separately rather than together, using something like Postman to test your backend in isolation and the mock server to test your front-end in isolation.
When merging into master
, you need to decide whether to squash the commits or not. Squashing means that the commits will be "squashed" into a single commit before being added. Make sure you think whether the full commit history is useful or not.
Once your PR has been merged remember to delete the remote branch on GitHub to minimise clutter.
Use a git merge
for updating your PR branch with updates from other branches like master
since rebasing does not play well with GitHub's PR tracking. The history will be made nice when the PR is merged into master
using a rebase.