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

Fix all custom rules not being applied when any rule is configured incorrectly #1683

Merged
merged 3 commits into from
Jul 17, 2017

Conversation

marcelofabri
Copy link
Collaborator

Continued from #1681

// cc @JamieEdge

@SwiftLintBot
Copy link

SwiftLintBot commented Jul 17, 2017

1 Warning
⚠️ Make sure that the docs are updated by running the Generate docs scheme.
12 Messages
📖 Linting Aerial with this PR took 0.35s vs 0.34s on master (2% slower)
📖 Linting Alamofire with this PR took 2.25s vs 2.34s on master (3% faster)
📖 Linting Firefox with this PR took 9.75s vs 10.03s on master (2% faster)
📖 Linting Kickstarter with this PR took 14.03s vs 13.98s on master (0% slower)
📖 Linting Moya with this PR took 0.63s vs 0.63s on master (0% slower)
📖 Linting Nimble with this PR took 1.27s vs 1.28s on master (0% faster)
📖 Linting Quick with this PR took 0.42s vs 0.41s on master (2% slower)
📖 Linting Realm with this PR took 1.95s vs 2.07s on master (5% faster)
📖 Linting SourceKitten with this PR took 0.85s vs 0.86s on master (1% faster)
📖 Linting Sourcery with this PR took 2.82s vs 2.87s on master (1% faster)
📖 Linting Swift with this PR took 9.92s vs 9.83s on master (0% slower)
📖 Linting WordPress with this PR took 9.68s vs 9.71s on master (0% faster)

Generated by 🚫 Danger

@marcelofabri
Copy link
Collaborator Author

Filled SR-5477 🙃

Maybe we should just not run this test on Linux?

@marcelofabri marcelofabri force-pushed the mf-custom-rule-exception-handling branch from 462d9a0 to 4e5d1b2 Compare July 17, 2017 11:55
@marcelofabri marcelofabri force-pushed the mf-custom-rule-exception-handling branch from 4e5d1b2 to 5d0bc74 Compare July 17, 2017 11:56
@codecov-io
Copy link

codecov-io commented Jul 17, 2017

Codecov Report

Merging #1683 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1683      +/-   ##
==========================================
+ Coverage   87.31%   87.34%   +0.03%     
==========================================
  Files         201      201              
  Lines       10007    10031      +24     
==========================================
+ Hits         8738     8762      +24     
  Misses       1269     1269
Impacted Files Coverage Δ
Source/SwiftLintFramework/Rules/CustomRules.swift 98.27% <100%> (+0.19%) ⬆️
...sts/SwiftLintFrameworkTests/CustomRulesTests.swift 93.89% <100%> (+0.97%) ⬆️

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 9edde88...5d0bc74. Read the comment docs.

@marcelofabri marcelofabri merged commit c6546f0 into master Jul 17, 2017
@marcelofabri marcelofabri deleted the mf-custom-rule-exception-handling branch July 17, 2017 12:27
@marcelofabri marcelofabri mentioned this pull request Jul 17, 2017
@jpsim
Copy link
Collaborator

jpsim commented Jul 17, 2017

This is great, especially the bit to annotate tests to not run on Linux. 😊

However, why not misconfigure a rule in a cross-platform compatible way, so that we can benefit from this test on Linux without hitting SR-5477 (thanks for filing that, btw).

@marcelofabri
Copy link
Collaborator Author

🤔

I didn't realize that there're other ways to fail inside RegexConfiguration. I'll open a new PR.

@JamieEdge
Copy link
Contributor

JamieEdge commented Jul 17, 2017 via email

@marcelofabri
Copy link
Collaborator Author

No worries at all - I had some free time and wanted to merge this because it's an important fix. Thank you for tackling it 😊

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