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

Extend SwiftLint to allow team-wide and project-specific configs together #1357

Closed
gretzki opened this issue Mar 15, 2017 · 7 comments
Closed

Comments

@gretzki
Copy link

gretzki commented Mar 15, 2017

With SwiftLint it's currently possible to specify such a shared config file via the --config option.
Having a project-specific .swiftlint.yml file at the root of the project, a merge of both configurations is already possible. Even if the merge function is not implemented yet, as stated in #676.

But having multiple project specific configurations at different locations inside the project and one global configuration specified, the merge function is not invoked as expected.
The following example should clarify the problem:

$PROJECT_ROOT /

  • A.swift
  • .swiftlint.yml
  • Database/.swiftlint.yml
  • Database/B.swift

$HOME /

  • corporation-wide-swiftlint.yml

When analysing the project with swiftlint lint --path $PROJECT_ROOT --config $HOME/corporation-wide-swiftlint.yml the merge function is invoked as follows:

Merge1)
$PROJECT_ROOT/.swiftlint.yml (as parameter)
$HOME/corporation-wide-swiftlint.yml (as self)

Merge2)
$PROJECT_ROOT/Database/.swiftlint.yml (as parameter)
$HOME/corporation-wide-swiftlint.yml (as self)

Expected behavior:
When linting B.swift I would expect the recursive merge to merge as follows:
Expected Merge2a)
$PROJECT_ROOT/Database/.swiftlint.yml (as parameter)
$PROJECT_ROOT/.swiftlint.yml (as self)

Expected Merge2b)
Merged config of merge2a (as parameter)
$HOME/corporation-wide-swiftlint.yml (as self)

@nigelflack
Copy link

Hi - I got an update on this since I commented on #676

I think #1357 and #1352 (my issue) are basically the same right? (although I hadn't noticed the --config option!)

@gretzki
Copy link
Author

gretzki commented Mar 15, 2017

Hi @nigelflack,
you're right. Mine issue and yours require the same technical adaptations.
The only difference is, that I propose an automatic merge of the specified config file with the project's config files, instead of (your issue) specifying all config files explicitly.

But yeah, same good ideas ;)

@gretzki
Copy link
Author

gretzki commented Mar 16, 2017

As I debugged SwiftLint I realized, that the project's config file is already being merged with the global one specified via the config option.
Since the merge function is not implemented yet, the global config is simply being replaced by the project's one. So solving #676 will solve my issue too :)

@gretzki gretzki closed this as completed Mar 16, 2017
@gretzki
Copy link
Author

gretzki commented Mar 16, 2017

Ok, this was closed a bit early.
There's still some issues when specifying a global configuration via the config file and having multiple configs inside the project. I will update the first comment to include the latest issue.

@jpsim
Copy link
Collaborator

jpsim commented May 23, 2017

Sorry I don't understand your last comment @gretzki, notably why merging configurations (#676) isn't sufficient for your needs. Could you please elaborate?

@gretzki
Copy link
Author

gretzki commented Jun 27, 2017

Hi @jpsim .
As stated above in "expected behavior", I expected a recursive merge of config files up the folder hierarchy. But that's just one possible way to go I guess, so it's also ok as it is.
Nevermind, I'll close this issue for now ;)

@gretzki gretzki closed this as completed Jun 27, 2017
@Aranoledur
Copy link

Aranoledur commented Jul 18, 2017

Hi, @gretzki! Have you debugged this merge call?
Merge1)
$PROJECT_ROOT/.swiftlint.yml (as parameter)
$HOME/corporation-wide-swiftlint.yml (as self)

I set a breakpoint at Configuration.merge and it's not hit at this case. Only Merge2 is called. So as I understand the logic for --configparameter is replace config file at --path or working directory.

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

4 participants