Consistent Code Style - Githubissues

maplibre / maplibre-navigation-ios

MapLibre Navigation SDK for iOS

Other

36 stars 29 forks source link

Consistent Code Style #37

Closed Patrick-Kladek closed 5 months ago

Patrick-Kladek commented 5 months ago

Description

As discussed in #33 we want to ensure a consistent code style. Therefore this PR adds SwiftLint and also SwiftFormat.

Tasks

[x] ~~Add Swiftlint Plugin (this is a precompiled binary to reduce build time)~~
[x] Add SwiftFormat, and let it auto configure and save results
[x] Run SwiftFormat automatically on commit + add Documentation how to enable it.

Infos for Reviewer

SwiftLint requires iOS13 and above, if that's a deal breaker I'll remove SwiftLint

Had to disable the following default rules as they were producing a lot of errors which need some re-architecture and while doing so might break something:

force_cast
function_body_length
identifier_name
cyclomatic_complexity
type_body_length
file_length
force_try

Now that SwiftLint runs again, how should we deal with the TODO annotations in the codebase that SwiftLint exposes?

Patrick-Kladek commented 5 months ago

@StephanPartzsch @boldtrn As discussed earlier this PR reformats the whole codebase. Can you please review it

Patrick-Kladek commented 5 months ago

SwiftLint Package requires iOS13, maybe we can raise an Issue there https://github.com/lukepistrol/SwiftLintPlugin?

Bildschirmfoto 2024-04-12 um 19 14 06

StephanPartzsch commented 5 months ago

Speaking of packages and SPM. Do we have in mind to add a line to the readme that SPM is now an additional option and not only carthage?

boldtrn commented 5 months ago

@StephanPartzsch, Carthage has been removed actually.

boldtrn commented 5 months ago

SwiftLint Package requires iOS13, maybe we can raise an Issue there https://github.com/lukepistrol/SwiftLintPlugin?

I general I think going to iOS13, if we have to is no problem. Doing this for tooling, I am not sure? Do we get enough benefit from SwiftLint that we believe this is worth it? Or should just run the auto format for now?

So SwiftLint introduces two possible issues so far:

The build command with -skipPackagePluginValidation
Raise iOS to 13

Patrick-Kladek commented 5 months ago

@boldtrn I've submitted a PR which drops the min version for SwiftLint, till this is merged we can use my fork to keep iOS12 supported.

I guess in September when Xcode 16 becomes mandatory, iOS13 becomes the last supported version, till then we can go ahead with my fork. Even if my changes are not accepted for half a year I can maintain the fork

Patrick-Kladek commented 5 months ago

SwiftLint Plugin was removed and is run on CI now. But we still don't have a solution on how to communicate back the results