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

Only lint Swift source files within the provided target #4406

Merged
merged 9 commits into from
Nov 24, 2022

Conversation

tonyarnold
Copy link
Contributor

When running with the SwiftPM build plugin provided in #4176, I noticed that regardless of which target was being built, the entire working directory was being linted.

This PR proposes to only lint Swift source files within the passed target by passing them as arguments to the build plugin process.

It also bails out early if the type of target it not suitable for linting, or there are no Swift source files to lint.

@jpsim
Copy link
Collaborator

jpsim commented Nov 21, 2022

Thanks for the PR, Tony!

This seems like a win at first glance, but may lead to some surprising results depending on how Xcode deals with diagnostics for files not in the currently selected target.

For example, what if you make change to a file that's not in your selected target but still covered by SwiftLint? The IDE wouldn't show you any diagnostics for that. Maybe it already doesn't?

It'd be surprising if Xcode says the project has no linter violations, only for then CI or a CLI invocation to say there are some.

If Xcode ignores non-target files regardless, then sure I'd say this is a nice win and we should do something like this.

@tonyarnold
Copy link
Contributor Author

Hey JP! 👋

For example, what if you make change to a file that's not in your selected target but still covered by SwiftLint? The IDE wouldn't show you any diagnostics for that. Maybe it already doesn't?

To be honest, I think that what you've described is exactly what I'd expect. I don't want files outside of my current build target to be assessed/linted by the current target's tooling. Listing of those files should be taken care of by whatever target those files are a part of.

CI should probably make use of a command plugin, rather than a build plugin? Especially if you want a customised/CI specific reporter, right?

@tonyarnold tonyarnold force-pushed the pass-paths-to-swiftlint branch 6 times, most recently from 8c4e196 to a3ed7ec Compare November 24, 2022 01:36
@jpsim
Copy link
Collaborator

jpsim commented Nov 24, 2022

I really don't have much experience with SwiftPM plugins. I'll merge this once you add a changelog entry describing the changes.

Thanks!

Signed-off-by: Tony Arnold <tony@thecocoabots.com>
Signed-off-by: Tony Arnold <tony@thecocoabots.com>
@jpsim jpsim merged commit f088bbd into realm:main Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants