Skip to content
This repository has been archived by the owner on Aug 11, 2024. It is now read-only.

Introduce Swiftlint #572

Merged
merged 14 commits into from
May 12, 2023
Merged

Introduce Swiftlint #572

merged 14 commits into from
May 12, 2023

Conversation

bfollington
Copy link
Collaborator

@bfollington bfollington commented May 4, 2023

Trying out a basic set of rules for catching mistakes like line length etc. everything is configured as a warning rather than an error right now so the build will never fail for lint reasons. I started with all the rules on and removed anything that was obviously in conflict with our existing style / rules.

We have ~500 lint warnings in the codebase atm and it would be good to fix them because github actions can only display a maximum of 10 linting errors per run (yeah, I know). So lint errors introduced in a PR won't actually show up in the UI until we clear the backlog of underlying lint issues.

The github action I used (made by the author of Swiftlint) purports to be able to scope the linting to only files changed in the PR but I get errors whenever I try to configure it as per their example.


Swiftlint can fix basic issues by running swiftlint --fix but we need to pick our moment carefully to avoid conflicts with in-flight work.

You can also choose to integrate swiftlint into Xcode as a build action (see this draft PR #573). If we gain enough confidence in rules I would be tempted to add a pre-commit hook that autofixes.

Copy link
Collaborator

@gordonbrander gordonbrander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Let's pick a time to merge this when it won't cause a bunch of disruption.

@bfollington bfollington merged commit 2a3a6d9 into main May 12, 2023
@bfollington bfollington deleted the 2023-05-04-swiftlint branch May 12, 2023 00:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants