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

Make forceExclude work with directly specified files #4609

Merged
merged 3 commits into from
Dec 22, 2022

Conversation

jimmya
Copy link
Contributor

@jimmya jimmya commented Nov 30, 2022

It seemed that --force-exclude didn't work when providing files directly.
.swiftlint.yml:

excluded:
  - ./**/*Test*

Directory contents:

 |-.swiftlint.yml
 |-Source
 | |-FooBar.swift
 | |-FooTest.swift
 | |-Foo.swift

When specifying a file that matches this excluded list:

➜ swiftlint --config .swiftlint.yml --force-exclude Source/FooTest.swift
Linting Swift files at paths Source/FooTest.swift
Linting 'Foo.swift' (1/2)
Linting 'FooBar.swift' (2/2)

Because it fell through the if statement it started linting all included files.

When specifying a file that doesn't match:

➜ swiftlint --config .swiftlint.yml --force-exclude Source/Foo.swift    
Linting Swift files at paths Source/Foo.swift
Linting 'Foo.swift' (1/1)

Expected result:

➜ swiftlint --config .swiftlint.yml --force-exclude Source/FooTest.swift                
Linting Swift files at paths Source/FooTest.swift
Error: No lintable files found at paths: 'Source/FooTest.swift'

This PR makes sure that this excluded list is taken into account when specifying the --force-exclude flag.

@SwiftLintBot
Copy link

SwiftLintBot commented Nov 30, 2022

18 Messages
📖 Linting Aerial with this PR took 1.1s vs 1.09s on main (0% slower)
📖 Linting Alamofire with this PR took 1.41s vs 1.39s on main (1% slower)
📖 Linting Brave with this PR took 7.49s vs 7.47s on main (0% slower)
📖 Linting DuckDuckGo with this PR took 3.0s vs 3.01s on main (0% faster)
📖 Linting Firefox with this PR took 9.55s vs 9.53s on main (0% slower)
📖 Linting Kickstarter with this PR took 10.26s vs 10.23s on main (0% slower)
📖 Linting Moya with this PR took 0.56s vs 0.57s on main (1% faster)
📖 Linting NetNewsWire with this PR took 3.25s vs 3.23s on main (0% slower)
📖 Linting Nimble with this PR took 0.63s vs 0.62s on main (1% slower)
📖 Linting PocketCasts with this PR took 7.59s vs 7.59s on main (0% slower)
📖 Linting Quick with this PR took 0.24s vs 0.24s on main (0% slower)
📖 Linting Realm with this PR took 11.95s vs 11.98s on main (0% faster)
📖 Linting SourceKitten with this PR took 0.45s vs 0.45s on main (0% slower)
📖 Linting Sourcery with this PR took 2.31s vs 2.32s on main (0% faster)
📖 Linting Swift with this PR took 4.48s vs 4.46s on main (0% slower)
📖 Linting VLC with this PR took 1.32s vs 1.32s on main (0% slower)
📖 Linting Wire with this PR took 8.98s vs 9.03s on main (0% faster)
📖 Linting WordPress with this PR took 11.27s vs 11.26s on main (0% slower)

Generated by 🚫 Danger

@jimmya
Copy link
Contributor Author

jimmya commented Dec 13, 2022

@jpsim could you perhaps have a look at this PR?

Copy link
Collaborator

@SimplyDanny SimplyDanny left a comment

Choose a reason for hiding this comment

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

The change seems reasonable to me. Thank you for the fix! I only have two small suggestions.

@jimmya
Copy link
Contributor Author

jimmya commented Dec 21, 2022

@SimplyDanny thanks for the review! I've implemented the requested change.

@SimplyDanny
Copy link
Collaborator

@jimmya: Thank you for taking care of the comments! Could you please rebase your changes once more to fix the failing tests?

@jimmya jimmya force-pushed the improvement/exclude-specified-files branch from cba8372 to c3e42b4 Compare December 22, 2022 19:02
@jimmya
Copy link
Contributor Author

jimmya commented Dec 22, 2022

@SimplyDanny thanks for approving. I've rebased and added my changelog entry

@SimplyDanny SimplyDanny merged commit eda0d92 into realm:main Dec 22, 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.

5 participants