Update PR template to be more explicit about expectations to check off everything

[flodnv]

Details

I have seen some confusions, hopefully this clears them up.
cc @Expensify/contributor-plus

Fixed Issues

https://expensify.slack.com/archives/C01SKUP7QR0/p1655892677571859

Tests

none needed

PR Review Checklist

this is what I am modifying

QA Steps

N/A

[flodnv]

Is something related to this line missing from the C+ checklist?

[sketchydroide]

hmm, seems you have two different patterns for the same thing:

1. . Check this off if it does not apply.
2. (check these off if they do not apply)

Not sure why we can't use (check these off if they do not apply) everywhere
And on the items that have subitems, it's not clear to me if we should only check the parent item, or the parent and all of the children.

[sketchydroide]

just a small question
Happy to aprove otherwise

[iwiznia]

Should we instead add this as instructions in the top? ie: instead of adding check this off if it does not apply to every item, let's just say so on the top

[sketchydroide]

Not sure, I mean, no body reads is a thing, and this makes sure it's read, I think I prefer to have it in every line

[parasharrajat]

I agree with @iwiznia. Why not add a subtitle to the PR Review Checklist title that says Check off the items that do not apply.

[mananjadhav]

With some of my projects where we don't have such a large checklist, we use the pattern of marking the item checked and striking through to confirm it doesn't apply. Example

[x] Fork the Postman Collection if you have made an edit in the API contract

This gives a clarity to the reviewer as well what the dev thought doesn't apply but probably should've been tested.

[puneetlath]

I also agree that adding this at the top would be less clutter than having it in each line. Especially because we'll add new lines and will have to remember to add this to those.

Alternatively, we could add a last checkbox of something like I have checked off every box in the PR author checklist, including those that don't apply to this PR)

[sketchydroide]

Oh I like that @puneetlath

[mallenexpensify]

With some of my projects where we don't have such a large checklist, we use the pattern of marking the item checked and striking through to confirm it doesn't apply.

I really like @mananjadhav's strikethrough idea because it helps differentiate items that were checked vs those that aren't applicable (where the box is still checked). Can anyone think of any downside to this approach?

[sketchydroide]

@mallenexpensify

I really like @mananjadhav's strikethrough idea because it helps differentiate items that were checked vs those that aren't applicable (where the box is still checked). Can anyone think of any downside to this approach?

it doesn't sound like a bad pattern, but I don't know how to apply the strikethrough without editing the text, and that can be a bit annoying to do in such a large list of things to check (and maybe leading to do it on the wrong line).

[flodnv]

I agree! Quite obvious in hindsight indeed! I redid the PR: #9698