Browse Source

Docs: Adds details to Pull Request Checklist (#18471)

* Docs: Adds details to Pull Request Checklist

* Docs: Changes text according to PR review comments

* Docs: Adds State Management guidlines in FrontEnd style guidelines

* Update CONTRIBUTING.md

Co-Authored-By: gotjosh <josue@grafana.com>

* Update CONTRIBUTING.md

Co-Authored-By: gotjosh <josue@grafana.com>

* Docs: Some minor text changes for better english

Co-Authored-By: gotjosh <josue@grafana.com>

* Docs: Some minor text changes for better english

Co-Authored-By: gotjosh <josue@grafana.com>

* Docs: Some minor text changes for better english

Co-Authored-By: gotjosh <josue@grafana.com>

* Docs: Some minor text changes for better english

Co-Authored-By: gotjosh <josue@grafana.com>

* Docs: Some minor text changes for better english

Co-Authored-By: gotjosh <josue@grafana.com>
Hugo Häggmark 6 years ago
parent
commit
27ea2bef2c
3 changed files with 40 additions and 37 deletions
  1. 32 1
      CONTRIBUTING.md
  2. 8 0
      style_guides/frontend.md
  3. 0 36
      style_guides/pull-request-review-checklist.md

+ 32 - 1
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
 

+ 8 - 0
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<Props> = { ... }
 // 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.

+ 0 - 36
style_guides/pull-request-review-checklist.md

@@ -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.