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

Local setting of SwiftLint #96

Closed
RayZhao1998 opened this issue Mar 19, 2022 · 6 comments · Fixed by #150
Closed

Local setting of SwiftLint #96

RayZhao1998 opened this issue Mar 19, 2022 · 6 comments · Fixed by #150
Labels
clarification needed Further information is requested

Comments

@RayZhao1998
Copy link
Collaborator

Now the Github Action's lint is strict mode but not local. Should we open strict mode local too?

And do we need adding a SwiftLint pre-commit hook to check before commit?

@lukepistrol
Copy link
Member

As already said on Discord, my opinion:

Locally I'd prefer to not have strict mode enabled since it's more convenient to test/implement things without fixing small syntax inconsistencies. Once you are ready to commit you should then resolve all the warnings.

But I'm open to other's opinions.

@lukepistrol lukepistrol added the clarification needed Further information is requested label Mar 19, 2022
@0xWDG
Copy link
Collaborator

0xWDG commented Mar 21, 2022

I think it is a good idea, to set the local environment to strict.
Otherwise people will create a PR, and SwiftLint will fail, but not locally.

while in development you can always use:

  • // swiftlint:disable RULENAME, // swiftlint:enable RULENAME
  • // swiftlint:disable:next RULENAME
  • // swiftlint:disable:previous RULENAME
  • // swiftlint:disable all
  • // swiftlint:enable all

Maybe it's a good idea to implement something like remove_todos.sh to check for // swiftlint:disable all, or // swiftlint:disable things, in case people forget them.

personally i think // swiftlint:disable RULENAME is placed intended, // swiftlint:disable all can never be intended.

@lukepistrol
Copy link
Member

Good point. I agree

@RayZhao1998
Copy link
Collaborator Author

RayZhao1998 commented Mar 21, 2022

What about adding a pre-commit hook to forbid commit if swiftlint --strict failed and keep local environment normal?

@0xWDG
Copy link
Collaborator

0xWDG commented Mar 21, 2022

What about adding a pre-commit hook to forbid commit if swiftlint --strict failed and keep local environment normal?

Yes and no,
If you make the local environment strict, then you'll see the errors in Xcode, otherwise only in the terminal.
personally i like it more if i see the errors in Xcode instead of the terminal.

@lukepistrol
Copy link
Member

A agree with @wdg. I think no one will commit a build containing errors. And even if so we still have the workflow as a safe guard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification needed Further information is requested
Projects
Status: 🏁 Complete
Development

Successfully merging a pull request may close this issue.

3 participants