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

[Heads Up] SwiftLint may soon support remote configurations, which won't work out of the box here #148

Open
mokagio opened this issue Jun 11, 2020 · 1 comment

Comments

@mokagio
Copy link
Collaborator

mokagio commented Jun 11, 2020

I noticed this PR in the SwiftLint repo, which adds support for parent, child, and remote configurations.

I'm experimenting with it here and noticed that a "stock" configuration of the plugin won't work if you have a remote configuration. The reason is the same as why nested configurations don't work, as per this issue.

To be specific, this config file

# .swiftlint.yml
parent_config: https://raw.githubusercontent.com/Automattic/swiftlint-config/6e2333f88294a03e4e72d287367e1fd62000b049/.swiftlint.yml
remote_timeout: 10.0

will result in this output by the plugin.

# Output of `danger pr`, with `switlint.verbose = true`
Using config file: .swiftlint.yml
Swiftlint will be run from /Users/gio/Developer/a8c/WordPress-iOS-Shared
Swiftlint will exclude the following paths: []
Swiftlint includes the following paths: []
linting with options: {:config=>".swiftlint.yml", :reporter=>"json", :quiet=>true, :pwd=>"/Users/gio/Developer/a8c/WordPress-iOS-Shared", :force_exclude=>true}
Swiftlint will lint the following files: 
Received from Swiftlint: {}

The workaround is the same as for the nested configuration, use the all_files flag.

I've been able to use it successfully here, with this Dangerfile

require 'git_diff_parser'

# The plugin looks into the .swiftlint.yml config to know in which folders to
# look for files to lint. Our config only has the URL of the remote config,
# though, so no file to lint would be found. This option makes the plugin lint
# all the files, and is "the same as [if] you were running `swiftlint` on the
# root folder".
# See
# https://github.com/ashfurrow/danger-ruby-swiftlint/tree/a8d6b3a6e82994f500411ad7acc2039d2d7409c7#usage
swiftlint.lint_all_files = true

# Despite using we want inline comments only in the files that changed in the
# PR, instead of a long wall of text with each of the files in the repo failing
# to lint. This extra code makes it possible.
# See
# https://github.com/ashfurrow/danger-ruby-swiftlint/tree/a8d6b3a6e82994f500411ad7acc2039d2d7409c7#usage
diff = GitDiffParser::Patches.parse(github.pr_diff)
dir = "#{Dir.pwd}/"
swiftlint.lint_files(inline_mode: true) { |violation|
  diff_filename = violation['file'].gsub(dir, '')
  file_patch = diff.find_patch_by_file(diff_filename)
  file_patch != nil && file_patch.changed_lines.any? { |line| line.number == violation['line']}
}

I don't think any action is required right now, but just wanted to give the team a heads up. 👌

Maybe when the SwiftLint feature ships a README update could help. In the meantime, this issue should do.

@ashfurrow
Copy link
Owner

@mokagio Huge props for opening this issue up 👏 No doubt it will help others. Agreed about next steps and waiting for the readme update. Hope you're well!

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