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

Update contributing guidelines, add a PR template #72

Merged
merged 4 commits into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!--

For more information on any of the below, please see our Contributing guidelines:
https://github.com/FormidableLabs/react-fast-compare/blob/master/CONTRIBUTING.md#before-submitting-a-pr

-->

## Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

## Checklist: (Feel free to delete this section upon completion)

- [ ] All tests are passing
kale-stew marked this conversation as resolved.
Show resolved Hide resolved
- [ ] Bundle size has not been significantly impacted
- [ ] The bundle size badge has been updated to reflect the new size
48 changes: 32 additions & 16 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,17 +1,25 @@
Contributing
============
# Contributing

Thanks for contributing!

## Before you contribute

This package is a fork of [fast-deep-equal](https://github.com/epoberezkin/fast-deep-equal). This library has added handling for React. Before contributing, _please make sure the issue relates directly to this library and not fast-deep-equals_.
This package is a fork of [fast-deep-equal](https://github.com/epoberezkin/fast-deep-equal). This library has added handling for React.
Before contributing, _please make sure the issue relates directly to this library and not fast-deep-equal's_.
kale-stew marked this conversation as resolved.
Show resolved Hide resolved

We encourage pull requests concerning:

* React features not handled in this library
* Integrating updates from `fast-deep-equal`. This unfortunately, now requires more manual work to use the comment blocks in `index.js` to figure out what to paste and where.
* Integrating tests from `fast-deep-equal`. This usually entails upgrading the `git`-based dependencies of `fast-deep-equal-git` and `npm`-published package of `fast-deep-equal` in `package.json:devDependencies`.
* Integrating updates from `fast-deep-equal`

kale-stew marked this conversation as resolved.
Show resolved Hide resolved
This, unfortunately, now requires more manual work. Use the comment blocks in `index.js`
to figure out what to paste and where.

* Integrating tests from `fast-deep-equal`

This usually entails upgrading the `git`-based dependencies of `fast-deep-equal-git` and
`npm`-published package of `fast-deep-equal` in `package.json:devDependencies`.

* Bugs in this library
* New tests for React
* Documentation
Expand All @@ -30,7 +38,7 @@ Install the project using `yarn` (which we've standardized on for development):
$ yarn install
```

`tl;dr` -- Everything you normally need to run is aggregated into:
**TL; DR:** -- Everything you normally need to run is aggregated into:

```sh
$ yarn run test
Expand Down Expand Up @@ -106,24 +114,32 @@ $ yarn -s compress
$ yarn size-min-gz
```

**Note**: If the min+gz size increases, please note it in the README. If it is a significant increase, please flag to your reviewers and have a discussion about whether or not the size addition is justified.
**Note**: If the min+gz size increases, please note it in the README. If it is a significant increase,
please flag to your reviewers and have a discussion about whether or not the size addition is justified.

## Before submitting a PR...

Before you go ahead and submit a PR, make sure that you have done the following:
... please make sure that you have done the following:

```sh
$ yarn run test
$ yarn run benchmark
```
1. Confirm that all checks are passing:

```sh
$ yarn run test
$ yarn run benchmark
```

2. Confirm we don't have any significant performance regressions (check out `master` for a baseline comparison on _your_ machine).

3. Confirm you aren't impacting our benchmark stats.
kale-stew marked this conversation as resolved.
Show resolved Hide resolved
If you _do_ affect the benchmark stats, please update the benchmark badge in the Readme by
* Following the steps outlined in [size](#size):
`yarn -s compress && yarn size-min-gz`
* Grabbing that output and replacing the current size in the bundle_img: (`https://img.shields.io/badge/minzipped%20size-<NEW_SIZE>%20B-flatgreen.svg`)

1. Everything must be correct / pass checks.
2. You should also check the benchmark stats and make sure that we don't have any significant performance regressions (check out `master` for a baseline comparison on _your_ machine).
- Please **do** update the README benchmark numbers for changes in your PR so that we have much easier discussion points _and_ our users get up-to-date information.

## Releasing a new version to NPM

_Only for project administrators_.
_Only for project administrators_

1. Run `npm version patch` (or `minor|major|VERSION`) to run tests and lint,
build published directories, then update `package.json` + add a git tag.
Expand Down