-
Notifications
You must be signed in to change notification settings - Fork 290
🔨 Update tools for development on Apple Silicon #334
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
Conversation
houndci-bot
left a comment
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.
Some files could not be reviewed due to errors:
Bad exclusion: script/sort.pl
Bad exclusion: script/sort.pl
houndci-bot
left a comment
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.
Some files could not be reviewed due to errors:
Bad exclusion: script/sort.pl
Bad exclusion: script/sort.pl
houndci-bot
left a comment
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.
Some files could not be reviewed due to errors:
Loading configuration from '.swiftlint.yml'
Loading configuration from '.swiftlint.yml' Invalid configuration for 'opening_brace'. Falling back to default. Linting Swift files at paths MasKitTests/Extensions/Bundle+JSON.swift Linting 'Bundle+JSON.swift' (1/1)
houndci-bot
left a comment
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.
Some files could not be reviewed due to errors:
Loading configuration from '.swiftlint.yml'
Loading configuration from '.swiftlint.yml' Invalid configuration for 'opening_brace'. Falling back to default. Linting Swift files at paths MasKitTests/Extensions/Bundle+JSON.swift Linting 'Bundle+JSON.swift' (1/1)
houndci-bot
left a comment
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.
Some files could not be reviewed due to errors:
Loading configuration from '.swiftlint.yml'
Loading configuration from '.swiftlint.yml' Invalid configuration for 'opening_brace'. Falling back to default. Linting Swift files at paths MasKitTests/Extensions/Bundle+JSON.swift Linting 'Bundle+JSON.swift' (1/1)
houndci-bot
left a comment
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.
Some files could not be reviewed due to errors:
Loading configuration from '.swiftlint.yml'
Loading configuration from '.swiftlint.yml' Invalid configuration for 'opening_brace'. Falling back to default. Linting Swift files at paths MasKitTests/Extensions/Bundle+JSON.swift Linting 'Bundle+JSON.swift' (1/1)
Hound supports only SwiftLint 0.27.0 from July 2018. The current release is 0.43.1 from last week. I've disabled SwiftLint in |
These appear to be a disagreement between Hound's older version of SwiftLint and the current version. |
|
@phatblat, is Jenkins healthy? It does look like builds have been successful there lately. I don't want to risk regressions to the Jenkins build, but I don't know what the healthy baseline should be. |
I would like to have a check fail when the linter returns errors. Could you split out the formatters into their own script? Then |
|
Sounds good. I'll split out I've rebased against |
|
Hrm... I'm not sure what the |
|
Oh, I think it's confused since you submitted this one from a fork. I'll override to get this big PR in before the other ones. @chris-araman now that you're a contributor, you can create branches right on the |
Ruby 2.4.3 is no longer in support and fails to compile with current tools. The
dangerandxcprettygems seem to run successfully on Ruby 3.0.0.The
swiftenvformula is not available in thehomebrew/coretap, so we now specify its tap askylef/formulae, per instructions at https://swiftenv.fuller.li/.An arm64
shellcheckbottle is not available becauseghcis not yet available for arm64 via Homebrew (Homebrew/homebrew-core#65997). I've addedshfmtin the meantime, as it is complementary. I'll be happy to addshellcheckback once it's available for arm64.Some of the scripts were indenting with 4 spaces, and others with 2. I decided to standardize on 2 for brevity, but am happy to change it if others disagree.
I thought it might be a good idea to install the Swift linters via Homebrew and remove our dependence on Mint. The linters weren't all available from Homebrew when first added to this project, but they are now. Mint is currently problematic on M1 Macs because it fails to link globally to
/usr/local/binwhen that path does not yet exist. Homebrew for M1 Macs defaults to/opt/homebrew/bin. One thing we lose here is the ability to pin to particular versions of the linters, as Homebrew only supports installing the "current" version. I think that's a fair trade-off.The Homebrew bundle lockfile isn't a typical lockfile in that it doesn't support version pinning, however it does add some value in that it provides evidence of a last-known-good state. However, it doesn't seem to handle removed or renamed brews/taps/mas entries. To work around that oversight, I've started wiping it during mas build bootstrapping. This allows us to maintain a clearer picture of the last-known-good Homebrew bundle state. However, I'd be happy to remove this "lockfile" and start passing
--no-lockinstead, if others feel it isn't meeting our needs.The
sortscripts were used when CocoaSeeds were still in use, but references to both have previously been removed.I've enabled the
swiftformatandswift-formatlinters in addition to the existingswiftlintlinter. Getting them to agree on certain rules took a bit of finesse.