-
Notifications
You must be signed in to change notification settings - Fork 279
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
🔨 Update tools for development on Apple Silicon #334
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.
Some files could not be reviewed due to errors:
Bad exclusion: script/sort.pl
Bad exclusion: script/sort.pl
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
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)
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)
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)
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
danger
andxcpretty
gems seem to run successfully on Ruby 3.0.0.The
swiftenv
formula is not available in thehomebrew/core
tap, so we now specify its tap askylef/formulae
, per instructions at https://swiftenv.fuller.li/.An arm64
shellcheck
bottle is not available becauseghc
is not yet available for arm64 via Homebrew (Homebrew/homebrew-core#65997). I've addedshfmt
in the meantime, as it is complementary. I'll be happy to addshellcheck
back 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/bin
when 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-lock
instead, if others feel it isn't meeting our needs.The
sort
scripts were used when CocoaSeeds were still in use, but references to both have previously been removed.I've enabled the
swiftformat
andswift-format
linters in addition to the existingswiftlint
linter. Getting them to agree on certain rules took a bit of finesse.