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.yml #383

Merged
merged 7 commits into from
Oct 17, 2020
Merged

Add .swiftlint.yml #383

merged 7 commits into from
Oct 17, 2020

Conversation

417-72KI
Copy link
Member

@417-72KI 417-72KI commented Oct 9, 2020

resolves #382

@417-72KI 417-72KI marked this pull request as draft October 12, 2020 04:26
@417-72KI 417-72KI marked this pull request as ready for review October 12, 2020 04:53
.swiftlint.yml Outdated
Comment on lines 14 to 16
identifier_name:
min_length:
warning: 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use excluded: to add the excluded words instead? (like id)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! fixed in 38131f0

@f-meloni
Copy link
Member

Thank you very much :)

@f-meloni
Copy link
Member

PS. I believe you will need to add the config file to the Dangerfile https://github.com/danger/swift/blob/master/Dangerfile.swift#L23

@417-72KI
Copy link
Member Author

As you said (#382 (comment)), SwiftLint should be executed in whole project.

Some of them are in the JSON, and I'm in favour of ignore them, while I would like to keep the Test files still at a quality that is in line with the rest of the code if possible.

But in current, SwiftLint verifies only on Sources.
I'm trying to fix by unsetting --path property on swiftlint, and it should not need to set config file (.swiftlint.yml file on project root will be read).

@f-meloni
Copy link
Member

@417-72KI what about using

let files = (danger.git.modifiedFiles + danger.git.createdFiles).filter { ($0.starts(with: "Tests") ||  $0.starts(with: "Sources")) && 
$0.fileType == .swift }

SwiftLint.lint(.files(files), inline: true)

?

@417-72KI
Copy link
Member Author

@f-meloni
Here is my solution (https://github.com/danger/swift/pull/383/files#diff-01136c42fecd3d9a2f9d247c6391859f063fcf4bdb445092e14ccbc8ed02bfcfR23)

or, do you mean to make it runs twice (for Sources and for Tests) ?

@f-meloni
Copy link
Member

@417-72KI The code I wrote is basically filtering the modified files to only include files in test and sources, but I'm totally fine with your solution. Thank you!

@f-meloni f-meloni merged commit 27e4894 into danger:master Oct 17, 2020
@417-72KI 417-72KI deleted the swiftlint branch October 18, 2020 15:28
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.

[discussion] Exclude Test folder on SwiftLint.
2 participants