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

Don't trigger discarded_notification_center_observer when the observer is returned #1581

Merged
merged 1 commit into from
Jun 2, 2017
Merged

Conversation

marcelofabri
Copy link
Collaborator

Fixes #1525

@SwiftLintBot
Copy link

12 Messages
📖 Linting Aerial with this PR took 0.36s vs 0.34s on master (5% slower)
📖 Linting Alamofire with this PR took 2.16s vs 2.17s on master (0% faster)
📖 Linting Firefox with this PR took 9.42s vs 9.35s on master (0% slower)
📖 Linting Kickstarter with this PR took 12.44s vs 12.35s on master (0% slower)
📖 Linting Moya with this PR took 0.65s vs 0.67s on master (2% faster)
📖 Linting Nimble with this PR took 1.24s vs 1.25s on master (0% faster)
📖 Linting Quick with this PR took 0.41s vs 0.42s on master (2% faster)
📖 Linting Realm with this PR took 1.93s vs 1.89s on master (2% slower)
📖 Linting SourceKitten with this PR took 0.82s vs 0.84s on master (2% faster)
📖 Linting Sourcery with this PR took 1.98s vs 1.96s on master (1% slower)
📖 Linting Swift with this PR took 8.84s vs 8.78s on master (0% slower)
📖 Linting WordPress with this PR took 9.22s vs 9.13s on master (0% slower)

Generated by 🚫 Danger

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1581      +/-   ##
==========================================
+ Coverage   84.24%   84.28%   +0.03%     
==========================================
  Files         192      192              
  Lines        9979    10000      +21     
==========================================
+ Hits         8407     8428      +21     
  Misses       1572     1572
Impacted Files Coverage Δ
...ules/DiscardedNotificationCenterObserverRule.swift 100% <100%> (ø) ⬆️

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 da7c0d9...9b0e259. Read the comment docs.

@marcelofabri marcelofabri merged commit 2045780 into realm:master Jun 2, 2017
@marcelofabri marcelofabri deleted the fix-1525 branch June 2, 2017 19:07
@jpsim
Copy link
Collaborator

jpsim commented Jun 2, 2017

Nice fix, although I worry that there are many more cases in which a valid use of addObserver that doesn't store to a variable or return, such as appending the result to an Array/Set/etc, among many other operations...

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