Add a comment in the icon component about how to view the icons in icomoon.
This is documentation. per the rules (https://www.notion.so/commapp/Diff-Review-Rules-addfc3a41b324c7d8ba636dd9dcefeba) adding ashoat.
Details
Details
go through the steps and make sure they make sense to you and anyone else on the team.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- arcpatch-D3398
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/SWMansionIcon.react.js | ||
|---|---|---|
| 12 ↗ | (On Diff #10273) | should I add the directory to be a bit more descriptive here, or do you think most folks will know to just search for AppIcons.json in the project? |
Comment Actions
Minor nit but if I remember correctly (cc @ashoat), the team generally avoids multi-line comments with /* and */ and instead prefers starting each line with //
Not sure of the reasoning or context, but I vaguely remember something like that being a "rule"
Comment Actions
Yeah I think this is generally reflective of the pattern that's emerged in the codebase, and it's probably better to stick to it. Not that it really matters...
| web/SWMansionIcon.react.js | ||
|---|---|---|
| 8–15 ↗ | (On Diff #10273) | Please keep all lines to max 80 chars width! |
| 12 ↗ | (On Diff #10273) | Probably doesn't hurt to put the full directory in |
Comment Actions
For multi-line comments IMO find them waaaay easier to read. Going to leave it here for now if it's not a big deal.
| web/SWMansionIcon.react.js | ||
|---|---|---|
| 8–15 ↗ | (On Diff #10273) | Sorry, my editor does it by default. I'm not sure why it didn't format it by default. |
| web/SWMansionIcon.react.js | ||
|---|---|---|
| 11–16 | Some nits:
| |
| web/SWMansionIcon.react.js | ||
|---|---|---|
| 13 ↗ | (On Diff #10307) | One of my nits from last time was not addressed:
|