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 option warnings-as-errors to lint & analyze #3371

Closed
wants to merge 2 commits into from

Conversation

Jeehut
Copy link
Collaborator

@Jeehut Jeehut commented Oct 2, 2020

Implementes #3312.

I tested that this works manually by building via swift build -c release and running .build/release/swiftlint --warnings-as-errors and it worked just as expected. Without that option, I had 1 warning & 1 error reported, with that options 2 errors were reported, so I can confirm it works. But I did not add any unit tests because there seem to be no unit tests for the CLI product, other options like --lenient seem also not to be unit tested.

Note that I did not mess with any tresholds (like lenient does), I think we could think about those things after seeing how people are using this new option, so I'd rather keep this simple for now.

@SwiftLintBot
Copy link

SwiftLintBot commented Oct 2, 2020

1 Warning
⚠️ This PR may need tests.
12 Messages
📖 Linting Aerial with this PR took 2.19s vs 1.91s on master (14% slower)
📖 Linting Alamofire with this PR took 2.69s vs 2.71s on master (0% faster)
📖 Linting Firefox with this PR took 9.13s vs 9.21s on master (0% faster)
📖 Linting Kickstarter with this PR took 15.24s vs 15.24s on master (0% slower)
📖 Linting Moya with this PR took 1.25s vs 1.3s on master (3% faster)
📖 Linting Nimble with this PR took 1.36s vs 1.35s on master (0% slower)
📖 Linting Quick with this PR took 0.62s vs 0.62s on master (0% slower)
📖 Linting Realm with this PR took 2.64s vs 2.64s on master (0% slower)
📖 Linting SourceKitten with this PR took 1.02s vs 1.06s on master (3% faster)
📖 Linting Sourcery with this PR took 7.09s vs 7.12s on master (0% faster)
📖 Linting Swift with this PR took 10.91s vs 10.82s on master (0% slower)
📖 Linting WordPress with this PR took 17.07s vs 16.89s on master (1% slower)

Generated by 🚫 Danger

@Jeehut Jeehut changed the title [WIP] Add option warnings-as-errors to lint & analyze Add option warnings-as-errors to lint & analyze Oct 2, 2020
@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2020

Codecov Report

Merging #3371 into master will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3371      +/-   ##
==========================================
- Coverage   90.48%   90.46%   -0.03%     
==========================================
  Files         417      417              
  Lines       20508    20524      +16     
==========================================
+ Hits        18557    18567      +10     
- Misses       1951     1957       +6     
Impacted Files Coverage Δ
Source/swiftlint/Commands/AnalyzeCommand.swift 0.00% <0.00%> (ø)
Source/swiftlint/Commands/LintCommand.swift 0.00% <0.00%> (ø)
...ource/swiftlint/Helpers/LintOrAnalyzeCommand.swift 0.00% <0.00%> (ø)
...tFramework/Rules/Lint/ValidIBInspectableRule.swift 98.64% <0.00%> (-1.36%) ⬇️
...tyle/MultipleClosuresWithTrailingClosureRule.swift 85.52% <0.00%> (+1.31%) ⬆️
...mework/Rules/Idiomatic/NoFallthroughOnlyRule.swift 100.00% <0.00%> (+1.92%) ⬆️
...ramework/Rules/Style/NoSpaceInMethodCallRule.swift 100.00% <0.00%> (+15.78%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da408b5...3cd207e. Read the comment docs.

@jpsim
Copy link
Collaborator

jpsim commented Nov 7, 2020

Closed as #3372 has been merged.

@jpsim jpsim closed this Nov 7, 2020
@jpsim jpsim deleted the cg-warnings-as-errors branch November 7, 2020 22:54
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.

4 participants