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

Skip/remove swiftlint task #149

Open
aiKrice opened this issue Jun 30, 2020 · 2 comments
Open

Skip/remove swiftlint task #149

aiKrice opened this issue Jun 30, 2020 · 2 comments

Comments

@aiKrice
Copy link

aiKrice commented Jun 30, 2020

Hello @ashfurrow . First of all, nice plugin et very nice huge work!
I would like to know if you plan to make the run of swiftlint not mandatory?

I ask the question because i really think that when Orta created Danger (maybe iam wrong), he designed it to be a tool that take an output, work on it and send it as comment on Github PR.
Because of the design of the plugin, we have to fill the binary path, then swiftlint is running. But its running twice because juste before, i have setup swiftlint as a buildphase in my xcodeproj. So to avoid it, i have to write something like "if i am not running on CI, ./Pods/swiftlint/swiftlint". But that mean i create a dependency on my local build and CI build.
I think this plugin should just have parameter to have a path/to/swiftlint_report and a parse method, maybe some options to filter and format the report and that's all.
Isn't it?
You can have a look on Orta's danger-junit plugin here : https://github.com/orta/danger-junit/ and on the image in attachement. I know that you are a big contributor and i just want to have your point of view on this.

I understand it could be a potential breaking change and I propose my help to create a new parameter that will prevent raising an exception if the binary is not in the path, and it will just parse the report path and thats it. If you agree, i can work on it :)

Screenshot 2020-06-30 at 23 53 17

@ashfurrow
Copy link
Owner

Hello! Thank you for the kind words and the informative issue. This change makes sense to me – I would be happy to review a PR adding this feature.

I would recommend adding a new command, rather than augmenting the existing swiftlint.lint_files command with more arguments. Something like swiftlint.process_report, with matching tests. Let me know if I can provide more guidance here – I'm looking forward to seeing what you come up with!

@aiKrice
Copy link
Author

aiKrice commented Jul 1, 2020

Excellent !. process_report sounds really good . I will propose something in the following days.
Stay tuned :)

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

No branches or pull requests

2 participants