diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3ab8517e97b..049997d7469 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,7 +31,38 @@ To setup a local development environment we recommend reading [Building Grafana - Add tests relevant to the fixed bug or new feature. -- Follow [PR and commit messages guidelines](#PR-and-commit-messages-guidelines) +### High level checks + +- [ ] The pull request adds value and the impact of the change is in line with [Backend](https://github.com/grafana/grafana/tree/master/pkg) or [Frontend](https://github.com/grafana/grafana/tree/master/style_guides). +- [ ] The pull request works the way it says it should do. +- [ ] The pull request closes one issue if possible and does not fix unrelated issues within the same pull request. +- [ ] The pull request contains necessary tests. + +### Low level checks + +- [ ] The pull request contains a title that explains it. It follows [PR and commit messages guidelines](#Pull-Requests-titles-and-message) +- [ ] The pull request contains necessary link(s) to issue(s). +- [ ] The pull request contains commits with messages that are small and understandable. It follows [PR and commit messages guidelines](#Pull-Requests-titles-and-message) +- [ ] The pull request does not contain magic strings or numbers that could be replaced with an `Enum` or `const` instead. + +#### Bug specific checks + +- [ ] The pull request contains `Closes: #Issue` or `Fixes: #Issue` in pull request description. + +### Frontend specific checks + +- [ ] The pull request does not increase the Angular code base. + > We are in the process of migrating to React so any increment of Angular code is generally discouraged. +- [ ] The pull request does not contain uses of `any` or `{}` without comments describing why. +- [ ] The pull request does not contain large React components that could easily be split into several smaller components. +- [ ] The pull request does not contain back end calls directly from components, use actions and Redux instead. + +#### Redux specific checks (skip if pull request does not contain Redux changes) + +- [ ] The pull request does not contain code that mutates state in reducers or thunks. +- [ ] The pull request uses helpers `actionCreatorFactory` and `reducerFactory` instead of traditional `switch statement` reducers in Redux. See [Redux framework](https://github.com/grafana/grafana/tree/master/style_guides/redux.md) for more details. +- [ ] The pull request uses `reducerTester` to test reducers. See [Redux framework](https://github.com/grafana/grafana/tree/master/style_guides/redux.md) for more details. +- [ ] The pull request does not contain code that accesses the reducers state slice directly, instead, the code uses state selectors to access state. ### Pull Requests titles and message diff --git a/style_guides/frontend.md b/style_guides/frontend.md index 18069183e66..be43738f6f3 100644 --- a/style_guides/frontend.md +++ b/style_guides/frontend.md @@ -12,6 +12,7 @@ Generally we follow the Airbnb [React Style Guide](https://github.com/airbnb/jav 1. [Refs](#refs) 1. [Methods](#methods) 1. [Ordering](#ordering) +1. [State mangement](#State-mangement) ## Basic rules @@ -85,3 +86,10 @@ static defaultProps: Partial = { ... } // bad static defaultProps = { ... } ``` + +## State mangement + +- Don't mutate state in reducers or thunks. +- Use helpers `actionCreatorFactory` and `reducerFactory` instead of traditional `switch statement` reducers in Redux. See [Redux framework](https://github.com/grafana/grafana/tree/master/style_guides/redux.md) for more details. +- Use `reducerTester` to test reducers. See [Redux framework](https://github.com/grafana/grafana/tree/master/style_guides/redux.md) for more details. +- Use state selectors to access state instead of accessing state directly. \ No newline at end of file diff --git a/style_guides/pull-request-review-checklist.md b/style_guides/pull-request-review-checklist.md deleted file mode 100644 index 2fd017386ea..00000000000 --- a/style_guides/pull-request-review-checklist.md +++ /dev/null @@ -1,36 +0,0 @@ -# Pull Request Review Checklist - -## High level checks - -- [ ] The pull request adds value and the impact of the change is in line with [Frontend Style Guide](https://github.com/grafana/grafana/blob/master/style_guides/frontend.md). -- [ ] The pull request works the way it says it should do. -- [ ] The pull request does not increase the Angular code base. - > We are in the process of migrating to React so any increment of Angular code is generally discouraged from. (there are a few exceptions) -- [ ] The pull request closes one issue if possible and does not fix unrelated issues within the same pull request. -- [ ] The pull request contains necessary tests. - -## Low level checks - -- [ ] The pull request contains a title that explains the PR. -- [ ] The pull request contains necessary link(s) to issue(s). -- [ ] The pull request contains commits with commit messages that are small and understandable. -- [ ] The pull request does not contain magic strings or numbers that could be replaced with an `Enum` or `const` instead. - -### Bug specific checks - -- [ ] The pull request contains only one commit if possible. -- [ ] The pull request contains `closes: #Issue` or `fixes: #Issue` in pull request description. - -## Frontend specific checks - -- [ ] The pull request does not increase the number of `implicit any` errors. -- [ ] The pull request does not contain uses of `any` or `{}` without comments describing why. -- [ ] The pull request does not contain large React component that could easily be split into several smaller components. -- [ ] The pull request does not contain back end calls directly from components, use actions and Redux instead. - -### Redux specific checks (skip if pull request does not contain Redux changes) - -- [ ] The pull request does not contain code that mutate state in reducers or thunks. -- [ ] The pull request uses helpers `actionCreatorFactory` and `reducerFactory` instead of traditional `switch statement` reducers in Redux. ([Redux framework](https://github.com/grafana/grafana/blob/master/style_guides/redux.md)) -- [ ] The pull request uses `reducerTester` to test reducers.([Redux framework](https://github.com/grafana/grafana/blob/master/style_guides/redux.md)) -- [ ] The pull request does not contain code that access reducers state slice directly, instead the code uses state selectors to access state.