Lowercase imports before sorting #1218
Conversation
|
Hmmm. These numbers seem to be a little more than I would expect.. |
|
The longer benchmarks are more reliable and well within margins (3% for swift stdlib, 6% for Realm), so I'm not concerned about those. |
|
I can fix the new issue by bumping that up a line if that works for this fix? |
Codecov Report
@@ Coverage Diff @@
## master #1218 +/- ##
=======================================
Coverage 81.89% 81.89%
=======================================
Files 175 175
Lines 8898 8898
=======================================
Hits 7287 7287
Misses 1611 1611
Continue to review full report at Codecov.
|
|
The simple fix for this case would be to sort those SourceKitten imports like this: |
|
I'm uneasy about this, since it changes our definition of what is "sorted". Going back to the conversation in Slack:
Although I can see how this can be interpreted as being okay with changing the definition of what sorted means, what I actually meant was that we should consider a list that's sorted either case sensitive or case insensitive to be sorted, and the rule still serves its purpose. |
|
But I guess the problem here is that allowing both would mean that it would be acceptable for your imports to not be "sorted" for the way you wanted. It doesn't seem like a single consumer would want both ascii sorting, and "normal" sorting to be acceptable. |
|
so maybe it's a matter of adding a |
|
I'm happy to do that if that's the way you want to go, but besides backwards compatibility I do think it's strange to even allow "sorting" to be determined by the ascii order of characters for this use case. |
|
@jpsim any solution here you'd be happy with? |
|
Alright, let's do this as-is. Could you please just update the changelog and merge? |
|
Changelog is updated here? Did you want me to add something else? |
Currently if you have imports that differ in case, they are not sorted as you would expect.
|
Moved it to the breaking section in the changelog |
|
Thanks! |
Currently if you have imports that differ in case, they are not sorted
as you would expect.
Fixes #1185