Reorder stdlib tests w/ suggested style #3847
Conversation
rhagenson
added
do not merge
There was a problem hiding this comment.
I think everything looks fine now @rhagenson , is there anything else you want to do before removing the do not merge label?
|
Everything here is as I originally wanted to do. Since this PR includes a change to the style guide, I suspect even brief discussion tomorrow during sync is in order. One related point that I did not approach in this PR is ordering test classes in the same way as calls to those classes within |
STYLE_GUIDE.md
Outdated
| ifdef windows then | ||
| test(_TestOnWindowsA) | ||
| test(_TestOnWindowsB) | ||
| test(_TestOnWindowsC) | ||
| end | ||
|
|
||
| // Tests below exclude windows and are listed alphabetically | ||
| ifdef not windows then | ||
| test(_TestOnAllSystemsExceptWindowsA) | ||
| test(_TestOnAllSystemsExceptWindowsB) | ||
| test(_TestOnAllSystemsExceptWindowsC) | ||
| end |
There was a problem hiding this comment.
This looks fine to be, but I'd like to see the guide specify the order of the ifdef for OS and not.
So for example should ifdef osx come before ifdef windows?
where should "not's" etc be ordered.
I'd like to see a little more specification there.
What the specification is, I don't care. That's all you @rhagenson.
rhagenson
force-pushed
the
reorder-tests
branch
from
7d80608 to
5aa6d8f
Compare
During review of #3819 it was suggested that, given the long list of tests for the "files" package, it would be helpful to have a style guide change for test ordering. My suggestion was to alphabetize tests along with distinct sections for system inclusion/exclusion. I sorted the stdlib as part of this PR so we may use actual changes as useful examples for conversations of style.
Please read the changes here to STYLE_GUIDE.md for the full explanation and examples of my suggested style.