-
-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Apple's GitHub Workflows linting script #42
Conversation
This reverts commit f5cdfb3.
@@ -1,4 +1,5 @@ | |||
import XCTest | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really? A spurious newline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually can't find out why this wasn't reported before. This is needed based on how swift-format behaves.
Did some tests locally, it would sometimes randomly emit a warning about this, and sometimes not.
I couldn't find out what makes it care about that line, and what makes it not. Tried different configurations as well
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm strongly against needing the newline, frankly... If swift-format
is demanding it, to me that's a bug in swift-format
. Imports should be grouped together unless there are a lot of them and they fall naturally into sectioning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are at a point of "it is what it is" with swift-format. I'd suggest you give up as well. The configuration that Apple repos use also has the same behavior IIRC.
I think it requires the new line because the next import is a @testable one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, obviously it does but to me it's visually jarring and completely pointless to have that gap. Why should @testable
imports be separated?
(I know, I know, swift-format
isn't gonna change... but there should really be more control.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must say there are a lot of ways in which I don't like the direction swift-format
has taken. For all its issues, swiftformat
always impressed me more with its flexibility.
The script is configured to respect a
.swiftformatignore
file which I'll need for Penny.swift-format might soon have this ability in itself: swiftlang/swift-format#873