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

Updated unloading of plugins with Deferred #87

Closed
wants to merge 5 commits into from

Conversation

johnmaguire
Copy link
Owner

@S0lll0s I made some changes and wanted to open a new pull request to see what you thought before I merged this. I tried to make it a little more linear as well. Let me know if you think I did something wrong or whether you think I could make it more readable. :) I'm starting to get the hang of Twisted after many hours of tweaking this code just right. :P

Blocking #58

@johnmaguire
Copy link
Owner Author

I'm also wondering if there are any other obvious places we should add Deferred to right now. One might be the CardinalBot.who method which accepts a callback currently. The constructor of plugins is obviously a no-go, but the setup() function theoretically could return one -- could there be any value in trying to handle that? Maybe not until we have a reason?

@johnmaguire
Copy link
Owner Author

Superseded by #86 again. :)

@johnmaguire
Copy link
Owner Author

This bouncing back and forth thing is kinda fun @S0lll0s :P

@johnmaguire
Copy link
Owner Author

Closing this since I'm moving forward with #90

@johnmaguire johnmaguire deleted the feature/async-plugin-close branch March 21, 2020 21:28
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.

2 participants