Add contributor docs / guides, add pr + issue templates #6
Conversation
RavenNumber of Findings: 0 |
Current coverage is 96.96% (diff: 100%)@@ master #6 diff @@
==========================================
Files 26 26
Lines 1186 1186
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 1150 1150
Misses 36 36
Partials 0 0
|
|
|
||
| __Please ask first__ before embarking on any significant pull request (e.g. implementing features, refactoring code, porting to a different language), otherwise you risk spending a lot of time working on something that the project's lead developers might not want to merge into the project. | ||
|
|
||
| Please adhere to the [Dart Style Guide][dart-style-guide] for all changes contained in your pull requests. |
| 5. Write tests for your changes. | ||
| 1. There are no exceptions. | ||
| 2. If your PR doesn't have accompanying test coverage - your code | ||
| will not get merged. |
There was a problem hiding this comment.
This wording isn't super friendly. Not saying we shouldn't require changes to be tested, but may be good to include something like: "if you're having trouble, reach out in your PR about how to best go about testing your changes."
| 2. If your PR doesn't have accompanying test coverage - your code | ||
| will not get merged. | ||
|
|
||
| 6. Locally merge the upstream master branch into your topic branch: |
There was a problem hiding this comment.
May want to note that this is only necessary if there are merge conflicts
|
|
||
| ```bash | ||
| pub run test -p content-shell --pub-serve=8081 test/over_react_test.dart | ||
| ``` |
There was a problem hiding this comment.
There are the VM tests that we want to run as well, but wouldn't get run using these instructions. We should just recommend using pub run dart_dev test.
| ## Developer Workflow | ||
|
|
||
| The `over_react` developer workflow couldn't be any more simple! | ||
|
|
There was a problem hiding this comment.
"To serve the docs/examples/whatever, run:"
|
|
||
|
|
||
|
|
||
| __POTENTIAL AREAS OF REGRESSION:__ |
There was a problem hiding this comment.
#supernit...
I think using markdown headers and normal casing makes this much more readable:
## Ultimate problem:
## How it was fixed:
## Testing suggestions:
## Potential areas of regression:
---
__FYA:__ @greglittlefield-wf @aaronlademann-wf @jacehensley-wf @clairesarsam-wf @joelleibow-wfUltimate problem:
How it was fixed:
Testing suggestions:
Potential areas of regression:
FYA:
@greglittlefield-wf@aaronlademann-wf@jacehensley-wf@clairesarsam-wf@joelleibow-wf
| unnecessary null-checking in your business logic. | ||
|
|
||
| Also, if you fail to provide an initial value for all the fields in | ||
| `state`, your component will fail to build at run-time. |
There was a problem hiding this comment.
This isn't really accurate—or maybe the wording is just off?
|
|
||
| | ||
|
|
||
| * __AVOID__ specifying more than one cascading prop setter on the same line. |
There was a problem hiding this comment.
This should be grouped with the first couple guidelines.
Also, would it make sense to have separate sections for guidelines around consuming components and guidelines around building components?
| @@ -19,7 +19,9 @@ | |||
| * __[DOM components and props](#dom-components-and-props)__ | |||
| * __[Building custom components](#building-custom-components)__ | |||
| * __[Component Boilerplates](#component-boilerplate-templates)__ | |||
| * __[Component Formatting](#component-formatting)__ | |||
There was a problem hiding this comment.
See my penultimate comment; may be good to have a "component formatting" set of guidelines in the first level of this list, and then "component best practices" under this "Building custom components" section.
|
@greglittlefield-wf all feedback addressed. |
|
+1 |
|
@leviwith-wf this is ready for merge. |
QA +10 Merging. |
|
RM +1 manually reviewed dependencies |
Add contributor docs / guides, add pr + issue templates
Add boolean prop naming readability hint
Pretty self-explanatory...
I added
CONTRIBUTING.md, along with a PR / ISSUE template, and an update to the main readme with a word about our frienddartfmt, and our versioning scheme.FYA: @jacehensley-wf @clairesarsam-wf @greglittlefield-wf @joelleibow-wf