-
Notifications
You must be signed in to change notification settings - Fork 301
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
fix linux tests formatting #45
Conversation
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.
LGTM, looks fine.
I can't seem to find where the "our rules" are defined so people know to follow them?
Do we need a .swiftformat
config file committed perhaps? ( https://github.com/nicklockwood/SwiftFormat#config-file )
#else | ||
return Int(pthread_self()) | ||
#endif | ||
#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS) |
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.
shouldn't we match the formatting that Xcode does by default? For this one, Xcode does
private var threadId: Int {
#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS)
return Int(pthread_mach_thread_np(pthread_self()))
#else
return Int(pthread_self())
#endif
}
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.
@weissi i have no opinion on the format we should use. just want to make sure its consistent. currently we mostly use swiftformat --self insert --patternlet inline --stripunusedargs unnamed-only --comments ignore
but happy apply other settings if the team prefers something else. just let me know or put a follow up PR to "fix"
motivation: use consistent formatting changes: update generate_linux_tests to match the formatting rules we use: `swiftformat --self insert --patternlet inline --stripunusedargs unnamed-only --comments ignore`
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.
cool, let's do this one
motivation: use consistent formatting
changes: update generate_linux_tests to match the formatting rules we use:
swiftformat --self insert --patternlet inline --stripunusedargs unnamed-only --comments ignore