Add `--lenient` CLI option to `lint` command #1364

aaroncrespo · 2017-03-18T21:44:08Z

No description provided.

aaroncrespo · 2017-03-18T21:44:30Z

see #1359 for previous discussion

aaroncrespo · 2017-03-18T21:44:42Z

#1322

aaroncrespo · 2017-03-19T05:11:29Z

newest danger release appears to be hanging bundle exec danger --verbose

SwiftLintBot · 2017-03-20T09:36:37Z

	12 Messages
📖	Linting WordPress-iOS with this PR took 18.65s vs 19.28s on master (3% faster)
📖	Linting swift with this PR took 17.88s vs 16.42s on master (8% slower)
📖	Linting Aerial with this PR took 0.66s vs 0.57s on master (15% slower)
📖	Linting SourceKitten with this PR took 1.96s vs 1.55s on master (26% slower)
📖	Linting Sourcery with this PR took 4.87s vs 3.73s on master (30% slower)
📖	Linting ios-oss with this PR took 27.52s vs 30.1s on master (8% faster)
📖	Linting Alamofire with this PR took 6.33s vs 4.8s on master (31% slower)
📖	Linting firefox-ios with this PR took 25.37s vs 30.54s on master (16% faster)
📖	Linting Nimble with this PR took 2.2s vs 3.38s on master (34% faster)
📖	Linting Quick with this PR took 0.71s vs 0.98s on master (27% faster)
📖	Linting realm-cocoa with this PR took 3.68s vs 4.98s on master (26% faster)
📖	Linting Moya with this PR took 0.8s vs 0.78s on master (2% slower)

Generated by 🚫 danger

aaroncrespo · 2017-03-20T23:25:58Z

@marcelofabri the CI issue is resolved

marcelofabri · 2017-03-21T20:43:20Z

CHANGELOG.md

@@ -3,136 +3,141 @@
 ##### Breaking

 * `variable_name` rule (`VariableNameRule`) is now `identifier_name`
-  (`IdentifierNameRule`) as it validates other identifiers as well.  
+  (`IdentifierNameRule`) as it validates other identifiers as well.


the trailing whitespace shouldn't be removed

marcelofabri · 2017-03-21T21:00:21Z

Source/swiftlint/Commands/LintCommand.swift

@@ -137,6 +150,8 @@ struct LintOptions: OptionsProtocol {
            <*> mode <| configOption
            <*> mode <| Option(key: "strict", defaultValue: false,
                               usage: "fail on warnings")
+            <*> mode <| Option(key: "lenient", defaultValue: false,
+                               usage: "succeed on errors")


I think this should have a better description, since it's not the only thing that this option does

"downgrades serious violations to warnings, and any warning threshold is disabled"

marcelofabri · 2017-03-21T21:00:40Z

Source/swiftlint/Commands/LintCommand.swift

+            return violations
+        }
+        return violations.map {
+            $0.severity == .error ? StyleViolation(ruleDescription: $0.ruleDescription,


can you split this into an if statement?

aaroncrespo

see updates for review

aaroncrespo · 2017-03-21T21:24:25Z

Source/swiftlint/Commands/LintCommand.swift

@@ -137,6 +150,8 @@ struct LintOptions: OptionsProtocol {
            <*> mode <| configOption
            <*> mode <| Option(key: "strict", defaultValue: false,
                               usage: "fail on warnings")
+            <*> mode <| Option(key: "lenient", defaultValue: false,
+                               usage: "succeed on errors")


"downgrades serious violations to warnings, and any warning threshold is disabled"

codecov-io · 2017-03-23T01:36:58Z

Codecov Report

Merging #1364 into master will decrease coverage by 0.11%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #1364      +/-   ##
=========================================
- Coverage   81.91%   81.8%   -0.12%     
=========================================
  Files         181     181              
  Lines        9076    9089      +13     
=========================================
  Hits         7435    7435              
- Misses       1641    1654      +13

Impacted Files	Coverage Δ
Source/swiftlint/Commands/LintCommand.swift	`0% <0%> (ø)`	⬆️

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 a3547aa...fbd9ad3. Read the comment docs.

aaroncrespo

done pending a change to help text

marcelofabri · 2017-03-23T19:15:29Z

Thanks for the patience!

NachoSoto · 2017-03-24T17:09:45Z

Hmm this isn't working for me

This is the end of the script:

Done linting! Found 14 violations, 0 serious in 122 files.
Command /bin/sh emitted errors but did not return a nonzero exit code to indicate failure

Somehow it says it found 0 serious violations, even though it did emit errors:

/Users/nachosoto/dev/corsac/#!$%^.swift:497:21: error: Trailing Semicolon Violation: Lines should not have trailing semicolons. (trailing_semicolon)

Note that I obviously don't use semicolons, this is just a test 😅

marcelofabri · 2017-03-24T17:16:06Z

@NachoSoto can you please open an issue to track this?

aaroncrespo · 2017-03-24T17:16:16Z

Looks like wrt Xcode builds I needed to do more than suppress severity and correct return status. Our use case is running this on a server. outside of Xcode so I didn't see this case. Need to suppress emitting of Xcode errors I think.

NachoSoto · 2017-03-24T18:05:06Z

#1391.

aaroncrespo · 2017-03-25T15:37:26Z

looks like the issue is

violations = LintCommand.applyLeniency(options: options, violations: **violations**)
reporter.report(violations: **currentViolations**, realtimeCondition: true)

fix should be

violations = LintCommand.applyLeniency(options: options, violations: violations)
currentViolations = LintCommand.applyLeniency(options: options, violations: currentViolations)
reporter.report(violations: currentViolations, realtimeCondition: true)

aaroncrespo · 2017-03-25T15:45:30Z

or something similar.

Add --lenient CLI option to lint command

338168d

hard wrap changelog

d0f74c5

marcelofabri requested changes Mar 21, 2017

View reviewed changes

aaroncrespo added 2 commits March 21, 2017 17:12

Undo white space trimming

fa46ab0

core review fixes.

edd493c

aaroncrespo commented Mar 21, 2017

View reviewed changes

aaroncrespo and others added 2 commits March 22, 2017 15:02

Merge branch 'master' into lenient

5a46c69

shorten the msg

1cedd40

aaroncrespo commented Mar 23, 2017

View reviewed changes

aaroncrespo and others added 3 commits March 22, 2017 21:47

Merge branch 'master' into lenient

9040fdc

Update CHANGELOG.md

cb7d94f

Merge branch 'master' into lenient

fbd9ad3

marcelofabri merged commit 90351da into realm:master Mar 23, 2017

aaroncrespo deleted the lenient branch March 23, 2017 19:16

jpsim mentioned this pull request Mar 23, 2017

Request: add --lenient lint parameter to only generate warnings #1322

Closed

NachoSoto mentioned this pull request Mar 24, 2017

Linting with --lenient is still emitting errors #1391

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `--lenient` CLI option to `lint` command #1364

Add `--lenient` CLI option to `lint` command #1364

aaroncrespo commented Mar 18, 2017

aaroncrespo commented Mar 18, 2017

aaroncrespo commented Mar 18, 2017

aaroncrespo commented Mar 19, 2017

SwiftLintBot commented Mar 20, 2017 •

edited

Loading

aaroncrespo commented Mar 20, 2017

marcelofabri Mar 21, 2017

marcelofabri Mar 21, 2017

aaroncrespo Mar 21, 2017

marcelofabri Mar 21, 2017

aaroncrespo Mar 21, 2017

aaroncrespo left a comment

aaroncrespo Mar 21, 2017

codecov-io commented Mar 23, 2017 •

edited

Loading

aaroncrespo left a comment

marcelofabri commented Mar 23, 2017

NachoSoto commented Mar 24, 2017 •

edited

Loading

marcelofabri commented Mar 24, 2017

aaroncrespo commented Mar 24, 2017

NachoSoto commented Mar 24, 2017

aaroncrespo commented Mar 25, 2017

aaroncrespo commented Mar 25, 2017

Add --lenient CLI option to lint command #1364

Add --lenient CLI option to lint command #1364

Conversation

aaroncrespo commented Mar 18, 2017

aaroncrespo commented Mar 18, 2017

aaroncrespo commented Mar 18, 2017

aaroncrespo commented Mar 19, 2017

SwiftLintBot commented Mar 20, 2017 • edited Loading

aaroncrespo commented Mar 20, 2017

marcelofabri Mar 21, 2017

Choose a reason for hiding this comment

marcelofabri Mar 21, 2017

Choose a reason for hiding this comment

aaroncrespo Mar 21, 2017

Choose a reason for hiding this comment

marcelofabri Mar 21, 2017

Choose a reason for hiding this comment

aaroncrespo Mar 21, 2017

Choose a reason for hiding this comment

aaroncrespo left a comment

Choose a reason for hiding this comment

aaroncrespo Mar 21, 2017

Choose a reason for hiding this comment

codecov-io commented Mar 23, 2017 • edited Loading

Codecov Report

aaroncrespo left a comment

Choose a reason for hiding this comment

marcelofabri commented Mar 23, 2017

NachoSoto commented Mar 24, 2017 • edited Loading

marcelofabri commented Mar 24, 2017

aaroncrespo commented Mar 24, 2017

NachoSoto commented Mar 24, 2017

aaroncrespo commented Mar 25, 2017

aaroncrespo commented Mar 25, 2017

Add `--lenient` CLI option to `lint` command #1364

Add `--lenient` CLI option to `lint` command #1364

SwiftLintBot commented Mar 20, 2017 •

edited

Loading

codecov-io commented Mar 23, 2017 •

edited

Loading

NachoSoto commented Mar 24, 2017 •

edited

Loading