Use HTML comments in PR template; Update suggested GIF record.

[sawyerh]

Changes

• Update the PR template to use HTML comments, so that they aren't rendered in the published PR request.
• Replace LICECap with Kap as the recommended GIF recorder. Kap is newer and more maintained. I don't think LICECap works on M1 Macbooks for instance.

[lorenyu]

do the html comments get included in the default merge commit message?

[sawyerh]

@lorenyu Just checked on another repo and Yes, they do

[lorenyu]

i see LoL. in that case this change seems like it kinda hides the problem from the PR reviewer 😅

[sawyerh]

@lorenyu Not sure I follow. I thought the intent of these comments was to explain to the PR author what each section of the PR description should include. The expectation (I thought) is that these comments could technically be removed by the author when they fill it out.

[lorenyu]

The expectation (I thought) is that these comments could technically be removed by the author when they fill it out.

yeah that's the expectation. i'm just saying that if the PR author didn't remove the placeholders from the description in the first place (like they should have), making them HTML comments might hide them from the PR reviewer but it doesn't prevent the comments from making their way to the merge commit message. having them visible at least would allow the PR reviewer to catch them and call out to the author "hey, please remove these placeholders from the description and merge commit".

i don't actually have a preference for/against HTML comments, i feel like the fact that the placeholders should be removed should be common sense, but from your screenshot it looks like it's still making it's way into merge commit messages, so it sounds like just need a conversation with engineers to clean up the merge commit message before merging

[sawyerh]

i feel like the fact that the placeholders should be removed should be common sense, but from your screenshot it looks like it's still making it's way into merge commit messages, so it sounds like just need a conversation with engineers to clean up the merge commit message before merging

@lorenyu Ah I see, I should have provided more of my reasoning behind this in the PR description. Yeah, from what I've observed, it's pretty common for engineers to leave these in the PR description. That could be fine if the repo settings are set to default the commit message to just the PR title. In that scenario, removing each section isn't technically necessary if these are HTML comments.

[lorenyu]

ah i see makes sense!