41 contribution guidestandards for new contributors#266
41 contribution guidestandards for new contributors#266
Conversation
|
Please set a versioning label of either |
jbkolze
left a comment
jbkolze
left a comment
There was a problem hiding this comment.
Looks good for onboarding and developing some consistent maintainability practices. Had a couple notes on expectations that haven't really been implemented as of yet.
|
|
||
| ## Checklist | ||
|
|
||
| - [ ] I ran `npm run lint` |
There was a problem hiding this comment.
This feels a little misleading when there are currently 24 errors detected by npm run lint. Feels like either the repo should be cleaned up first or some clarification should be added on what the expectations are.
| ## Checklist | ||
|
|
||
| - [ ] I ran `npm run lint` | ||
| - [ ] I ran `npm test` or documented why tests were not needed |
There was a problem hiding this comment.
Similar to linting, no tests currently exist outside of a few for gw-merge, so a contributor is going to have a hard time determining what the expectation is. Unit tests with RTL/Vitest, accessibility tests, etc.? There's not really a baseline to work from.
There was a problem hiding this comment.
Is there a tangible benefit to having the contribution guide included in the library docs as opposed to just linking to CONTRIBUTING.md? As usual, I worry a bit about having multiple sources of truth. There's already a little discrepancy in the language related to linting.
| ## Coding standards | ||
|
|
||
| - Use Prettier formatting. A Husky pre-commit hook runs `lint-staged`, which applies Prettier to staged `js`, `jsx`, `json`, `css`, and `md` files. | ||
| - Run `npm run lint` before opening a pull request. ESLint is configured for the React codebase and should stay clean. |
There was a problem hiding this comment.
Same as in PR template -- I think we either need to clean up the repo or clarify expectations. Is the first person to contribute expected to clean up all of the existing issues?
2 participants
From #41
Covers via reference to doc and wording in quick start
Sets up other expectations based on current progress / dev on Groundwork