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

Remove @IBOutlet and @IBInspectable from UnusedDeclarationRule #3184

Merged
merged 2 commits into from
Nov 5, 2020

Conversation

keith
Copy link
Collaborator

@keith keith commented Apr 23, 2020

This list was intended to be things that could have side effects from
being in IB, or just being dynamic, and therefore we cannot tell if they
are used or not. In the case of the 2 attributes I'm removing, they
could be used in IB, but if they aren't used in code, they should still
be considered dead.

@keith
Copy link
Collaborator Author

keith commented Apr 23, 2020

Note making this change for folks is not entirely safe. If you remove an IBInspectable definition, without removing it from IB, it just warns in the console, but if you remove an IBOutlet without removing it from IB, it crashes.

@SwiftLintBot
Copy link

SwiftLintBot commented Apr 23, 2020

12 Messages
📖 Linting Aerial with this PR took 0.78s vs 0.8s on master (2% faster)
📖 Linting Alamofire with this PR took 0.99s vs 1.0s on master (1% faster)
📖 Linting Firefox with this PR took 3.76s vs 3.73s on master (0% slower)
📖 Linting Kickstarter with this PR took 6.44s vs 6.42s on master (0% slower)
📖 Linting Moya with this PR took 0.5s vs 0.48s on master (4% slower)
📖 Linting Nimble with this PR took 0.59s vs 0.6s on master (1% faster)
📖 Linting Quick with this PR took 0.33s vs 0.33s on master (0% slower)
📖 Linting Realm with this PR took 1.33s vs 1.34s on master (0% faster)
📖 Linting SourceKitten with this PR took 0.49s vs 0.5s on master (2% faster)
📖 Linting Sourcery with this PR took 2.57s vs 2.6s on master (1% faster)
📖 Linting Swift with this PR took 8.19s vs 8.19s on master (0% slower)
📖 Linting WordPress with this PR took 7.32s vs 7.23s on master (1% slower)

Generated by 🚫 Danger

@keith keith marked this pull request as ready for review April 27, 2020 14:59
@keith
Copy link
Collaborator Author

keith commented Nov 2, 2020

@jpsim what do you think about this?

Copy link
Collaborator

@jpsim jpsim left a comment

Choose a reason for hiding this comment

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

I think it makes sense to do this. Can you please add a change log entry and merge yourself when CI is green? Thanks!

@codecov-io
Copy link

codecov-io commented Nov 5, 2020

Codecov Report

Merging #3184 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3184   +/-   ##
=======================================
  Coverage   90.50%   90.51%           
=======================================
  Files         420      420           
  Lines       20555    20555           
=======================================
+ Hits        18604    18605    +1     
+ Misses       1951     1950    -1     
Impacted Files Coverage Δ
...ntFramework/Rules/Lint/UnusedDeclarationRule.swift 87.73% <100.00%> (ø)
...iftLintFramework/Extensions/String+SwiftLint.swift 94.54% <0.00%> (-1.82%) ⬇️
...tFramework/Rules/Lint/ValidIBInspectableRule.swift 100.00% <0.00%> (+1.35%) ⬆️
...ftLintFramework/Rules/Lint/PrivateOutletRule.swift 100.00% <0.00%> (+3.84%) ⬆️

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 e6df1d3...170bd03. Read the comment docs.

@keith keith merged commit 51084ad into master Nov 5, 2020
@keith keith deleted the ks/remove-unused-attrs branch November 5, 2020 16:21
keith added a commit that referenced this pull request Nov 16, 2020
The original implementation wasn't tested so it regressed.

#3184
keith added a commit that referenced this pull request Dec 3, 2020
The original implementation wasn't tested so it regressed.

#3184
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