docs: update contributing documentation and PR template #1411
Conversation
| <!-- check the items below that will be completed prior to merge. | ||
| strikethrough any item text that does not apply to this PR. --> | ||
|
|
||
| - [ ] :ok_hand: design updates will be Garden Designer approved (add the designer as a reviewer) |
There was a problem hiding this comment.
Why the switch to future tense?
There was a problem hiding this comment.
We want the PR author to check these items when the PR is submitted for review (aka, "I've thought of the items needed to open a PR"). We've seen folks confused about holding off on checkmarks because they haven't secured design review.
| - [ ] :guardsman: includes new unit tests | ||
| - [ ] :memo: tested in Chrome, Firefox, Safari, Edge, and IE11 | ||
| - [ ] :wheelchair: tested for [WCAG 2.1](https://www.w3.org/TR/WCAG21) AA compliance | ||
| - [ ] :memo: tested in Chrome, Firefox, Safari, and Edge |
There was a problem hiding this comment.
Should we specify versions (like "latest-2" or something?)
There was a problem hiding this comment.
The target is specified. Maybe we should link? (Although I'm not super keen about cluttering the PR template with raw markdown links.)
| - [ ] :guardsman: includes new unit tests | ||
| - [ ] :memo: tested in Chrome, Firefox, Safari, Edge, and IE11 | ||
| - [ ] :wheelchair: tested for [WCAG 2.1](https://www.w3.org/TR/WCAG21) AA compliance |
There was a problem hiding this comment.
Should we specify browser-AT combinations expected to work at minimum, or is the expectation that we would test all combinations.
| - [ ] :guardsman: includes new unit tests | ||
| - [ ] :memo: tested in Chrome, Firefox, Safari, Edge, and IE11 | ||
| - [ ] :wheelchair: tested for [WCAG 2.1](https://www.w3.org/TR/WCAG21) AA compliance |
There was a problem hiding this comment.
Wondering if it is worth noting here (or somewhere else so we don't make this list hard to read, maybe an asterisk?) that as a general rule, we stay away from adding one-off workarounds to account for browser/AT bugs?
There was a problem hiding this comment.
I think that could be an excellent addition to thedevelopment.md.
Description
Adds clarifying text for code contribution and refines the PR checklist. The new PR checklist can be viewed here.