-
Notifications
You must be signed in to change notification settings - Fork 16
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
Added listener remove functions #54
base: master
Are you sure you want to change the base?
Conversation
I need to mention that dokka generates the .html files based on the system path separator.. which is bad, i got 99 files of diff after generating the docs on windows (my fedora is broken :/), is there any config for that? |
Replaying to your comment @Grohden If the task is being 'listened' at the time of the removal.. what should happen? What do you mean by "task is being listened"? If a task is currently added as an active listener? We should really remove all instances when they're the same? Yes Should we use predicates instead of instances as param? (i personally prefer predicates, the dev has more control over it) Predicates are great, but in this case, not required, right? Listeners will be object instances so we can run Is there any thread restriction to remove a listener?
With that in mind, I am leaning towards waiting to merge this PR (since it's a nice-to-have and not required since we are using weak references) until we have unit testing setup and this PR can be fully tested. Agree? Also, we need some tests on Wendy and examples on the example app.. Agree. That should be the top priority at this time. the example app i don't have ideas for the remove functions uses The example app used to exist for me to test that Wendy works while not yet having unit testing setup. Because unit testing is a high priority now, the example app will exist to show off best practices and give some example code that people can use to play around with the library. With that in mind, I do not believe we need to add functionality to the example app to remove listeners. |
Thanks for the PRs, @Grohden! You are helping to make Wendy better and better. |
I mean, if you write a listener that removes itself on one of its callbacks. (i do not expect someone to do this.. but ¯\(ツ)/¯)
Agreed, i'm not in a hurry for this |
This should close #48
I added two listener removal functions.. but i think we need to discuss some situations
Also, we need some tests on Wendy and examples on the example app..
I think that testing needs to be done after #50 ... and the example app i don't have ideas for the remove functions uses