dt-mergebot icon indicating copy to clipboard operation
dt-mergebot copied to clipboard

Suggest the required form

Open jablko opened this issue 5 years ago • 3 comments

Putting this out there for feedback.

Iterate on #224. Show the differences from the required form as suggestions. Like #224, only shows freshly introduced differences, not preexisting diffs.

jablko avatar Nov 24 '20 16:11 jablko

Seems reasonable overall, but given that this also does some deeper changes, I'll wait until the new code gets merged (#234 and a following PR to change the stale timelines). (Sorry that it took so long, and it might take more time yet since it's thanksgiving week, so it's hard to find people to review stuff...)

All good, thank you for the reviews!

Also, IIUC, then this supersedes #224, right? It's more extreme in what it does, but I like the approach of having proper feedback rather than rely on the short comment lines in the welcome message. So if it does, then should #224 be closed?

Great! Done :heavy_check_mark:

Also, I didn't see any tests for this?

Done. I added DefinitelyTyped/DefinitelyTyped#49639

jablko avatar Nov 26 '20 17:11 jablko

Looks closer overall. Two things in addition to the above:

  • Can you add a test that adds (hopefully also removes) lines? It's fine to just make a copy of the test you added and hand-edit it.

  • There's a complaint now about PR's ending up in the "Other" column when the PRs don't have the expected header -- instead of making it go to "Needs Author Action" with a comment saying what's wrong. It's not really related to this PR, but if you see a quick way to add it, it would be better than me making a separate PR which will probably have some bad conflicts with this one. If you don't get to it, I might do so as commits on top of this PR.

elibarzilay avatar Nov 30 '20 23:11 elibarzilay

Can you add a test that adds (hopefully also removes) lines? It's fine to just make a copy of the test you added and hand-edit it.

Done: 4183e39396f130778633aa68399fafca8a57ba46

I hand-edited another file in the existing fixture. The bot adds the line that I hand-removed.

jablko avatar Dec 03 '20 16:12 jablko