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

Lint on Build (Xcode) #573

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Lint on Build (Xcode) #573

wants to merge 2 commits into from

Conversation

bfollington
Copy link
Collaborator

@bfollington bfollington commented May 4, 2023

Add a build step that (if available on the system) runs swiftlint. All linting errors appear in the Xcode issues panel.

e.g.

image

@bfollington bfollington changed the base branch from main to 2023-05-04-swiftlint May 4, 2023 02:16
@bfollington bfollington mentioned this pull request May 4, 2023
bfollington added a commit that referenced this pull request May 12, 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.
Base automatically changed from 2023-05-04-swiftlint to main May 12, 2023 00:36
@bfollington bfollington marked this pull request as ready for review May 15, 2023 23:29
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.

1 participant