Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update weak delegate rule to only match delegate suffix #923

Merged
merged 6 commits into from
Dec 3, 2016

Conversation

philwebster
Copy link

The rule was warning on lines like the following line in our tests:

var createActivityDelegateMethodCalled = false

I'm not sure if anyone names their delegates with delegate prefixed or in the middle (which this change would miss), but this seems like a reasonable modification.

@jpsim
Copy link
Collaborator

jpsim commented Dec 2, 2016

Can you please add a changelog entry? Probably under "Bug Fixes" would be best.

@AliSoftware this seem reasonable to you?

@philwebster
Copy link
Author

Done. Let me know if there's anything else!

@AliSoftware
Copy link
Contributor

👍

@marcelofabri
Copy link
Collaborator

Maybe we could add an entry to nonTriggeringExamples to make sure we don't break this in the future?

@jpsim
Copy link
Collaborator

jpsim commented Dec 3, 2016

Looks good @philwebster, but please ping me once Travis has had a chance to catch up, and as long as everything's green we'll merge.

@codecov-io
Copy link

codecov-io commented Dec 3, 2016

Current coverage is 83.23% (diff: 100%)

Merging #923 into master will not change coverage

@@             master       #923   diff @@
==========================================
  Files           114        114          
  Lines          5136       5136          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           4275       4275          
  Misses          861        861          
  Partials          0          0          

Powered by Codecov. Last update f243e0e...340f78c

@jpsim jpsim merged commit c72efc3 into realm:master Dec 3, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants