Skip to content
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

Add SwiftLint to codebase #86

Closed
okwasniewski opened this issue Oct 30, 2023 · 3 comments · Fixed by #90
Closed

Add SwiftLint to codebase #86

okwasniewski opened this issue Oct 30, 2023 · 3 comments · Fixed by #90
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@okwasniewski
Copy link
Owner

The goal of this issue is to add SwiftLint to the codebase to make code more consistent.

https://github.com/realm/SwiftLint

@okwasniewski okwasniewski added enhancement New feature or request good first issue Good for newcomers labels Oct 30, 2023
@Garfeild
Copy link
Contributor

Garfeild commented Oct 30, 2023

I can help with that. Would you be up to add Github Action to prevent merging inconsistent code? Usually it better to have several places to enforce linting, as contributors can bypass it on their local machine. In my experience, Xcode project script + CI check works great.

@okwasniewski
Copy link
Owner Author

@Garfeild Project script + CI running on every PR would be ideal setup 👍🏻

@Garfeild
Copy link
Contributor

Garfeild commented Nov 1, 2023

I have tried to integrate SwiftLint as plugin. There are some pros and cons with such integration.

Pros:

  • It shows warning/errors during compilation time in Xcode directly.
  • It's easy to share with any contributor and harder to disable.

Cons

  • It lints whole project and couldn't lint only changed files. This is not visible for current state of the project (takes ~0.25s).
  • Until we fix all warnings, build log will be polluted with them. Easy to miss important warnings.
    • After I cleaned up everything, it was 4 warnings.

Personally, I think it gives us good start and we can ignore first problem and mitigate second. And I would still introduce CI check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants