Underscore usage tests - reorganization#3
Conversation
…o UnderscoreStatic.
|
The diff for this unfortunately isn't the greatest. I tried not moving the OOP section to see if that would help but surprisingly that made it way worse (see underscore-tests-usage-tests-cleanup...reubenrybnik:underscore-tests-usage-tests-cleanup-b-2). Potential strategies to work around this include reviewing each commit individually (each addresses a section of tests) or requesting that I make a separate PR for each commit or so (I'll probably include two commits in a few of them). |
jgonggrijp
left a comment
There was a problem hiding this comment.
Reviewing the commits one by one (good suggestion!) and using the browser's in-page search function did the trick. In the red sections, I checked that each test also appeared in a green section, while in the green sections, I checked that each test was in a context that seemed to make sense. I couldn't find any mistake.
I've made a few comment change suggestions, but they are all very non-critical.
This might have been the most mechanical, repetitive review I've ever done, but it was totally worth it. I'm glad that we could partition this PR in such a way that the most complicated part could be done mechanically.
I look forward to giving the final thumbs-up back at the upstream repository.
Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com>
Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com>
Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com>
Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com>
…itions Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com>
|
Thanks for all the effort you put into this one @jgonggrijp, I hope it wasn't too tedious and I'm definitely open to suggestion on how such changes might be made easier to review in the future! I'm happy that the multiple PRs you recommended helped somewhat at least, and I'm also glad to hear that I didn't make any major slip-ups 😅 |
|
The only commits that aren't verified co-authored changes are pretty trivial, so I'm going to assume that this is good to go and merge this now. If I missed anything or made a change you're not happy with @jgonggrijp, I'll be happy to toss a quick update into the upstream PR. |
…rray Tests - Usage Tests Cleanup by @reubenrybnik * Underscore usage tests - addition, removal, and labeling of tests (#1) * Removing the EnumerableKey type alias because DT linting is not good at dealing with it between different versions of TypeScript. (cherry picked from commit abe1df7) * Adding comments that describe and anchor tests, changing a few tests slightly, and removing some tests. * Adding some chain tests. * Fixing spelling error. Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> * Updating the mixin test to properly augment Underscore types. * Removing an unnecessary generic type specification. * Wrapping a comment. * Referencing DefinitelyTyped#33479 in more places. Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> * Underscore usage tests - type assertions (#2) * Adding type assertions to usage tests. * Minor updates to the bind test. * Removing comments for chain tests that are already in the chain tests section of the file and updating the chain tests header. * Breaking a function into multiple lines for better readability. * Adding a separate toArray test that infers a more interesting type than any[]. * Replacing a movie reference with a more generic example. * Make indentation more consistent. Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> * Revert "Replacing a movie reference with a more generic example." This reverts commit 49c0a79. Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> * Underscore usage tests - reorganization (#3) * Removing the combinatorial tests label for types. * Making to the types section of tests. * Making updates to the collections section of tests. * Making updates to the arrays section of tests. * Making updates to the functions section of tests. * Making updates to the objects section of tests. * Making updates to the utility section of tests. * Making updates to the OOP section of tests and adding an OOP header to UnderscoreStatic. * Making updates to the chaining section of tests. * Adding a missing word to a sentence. Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> * Changing "sorting" to "order" when describing sortedIndex Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> * Switching "values its keys" to "vice versa" Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> * Removing an inaccurate "no-op" from a description Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> * Splitting up comments between calling mixin and augmenting type definitions Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> * Removing an extra memoize header. * Changing "TSC" to "TypeScript." Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com> Co-authored-by: Julian Gonggrijp <dev@juliangonggrijp.com>
This is the third of three PRs discussed in DefinitelyTyped#47543. It moves tests around.