bazz1tv / ayumi

My lovely IRC bot :sparkles: :turtle: :mailbox: :sparkles:
0 stars 1 forks source link

Plugin files are loaded only when a bot is initialized #96

Open TheNotary opened 8 years ago

TheNotary commented 8 years ago

This means loading the files (which we just want to test regarding #93) requires a config file and redis.

In other words because ayumi/bot.rb is where all the plugins are required from and they're only required when the bot object is instantiated, testing is more difficult and requires additional overhead.

1) Therefor I recommend moving away from that hoisting strategy and possibly pushing that code into lib/ayumi if possible.

2) Further I would try to get away from requiring in plugin .rb files based on a config file and instead require them in based on whether they exist on the FS in say, a plugin specific folder.

In the mean time, I'm just going to manually require in plugin files that I'm testing from lib/ayumi since I don't know the extent of the hoisting situation that exists presently.

bazz1tv commented 8 years ago

Let me verify I understand you. You want to be able to instantiate the bot independent of the plugins/database?

1) Therefor I recommend moving away from that hoisting strategy and possibly pushing that code into lib/ayumi if possible.

I'll look into it. I agree with this.

2) Further I would try to get away from requiring in plugin .rb files based on a config file and instead require them in based on whether they exist on the FS in say, a plugin specific folder.

I actually personally ventured away from that solution because I do not want the existence of a plugin to imply it should be loaded into the bot. Would reversing the situation satisfy you? In other words, loading all plugins from a folder unless the config strictly denies them? That would require adding a new per-plugin-config-option (disable), which is a good idea in itself.

In any case, a slightly related issue is that I want plugins to throw an error if "required configuration settings" haven't been specified #85

TheNotary commented 8 years ago

Let me verify I understand you. You want to be able to instantiate the bot independent of the plugins/database?

Not exactly. The bot isn't at all within the scope of my commits (I strictly added unit tests for the Game class). The bot is currently coupled to the plugins (it's responsible for loading them). That means you can't do tests on a plugin without first instantiating the bot. Coupling is generally bad. I didn't write any tests for the bot, just unit tests on the Game process so you could see why I made the refactor to it I made. Do you get why I returned those symbols from the game upon guess! now? Those are really easy to write assertions against as demonstrated in the spec file.

I do not want the existence of a plugin to imply it should be loaded

You could make it a feature to turn on/ off plugins over IRC and store that state information in the db. Is that the feature you're after? I might not understand your objective here. If that's the case then I would certainly roll with the blacklisting pattern you described, but I'd still recommend loading all the plugin code from lib/ayumi.rb and not do so depending on reading a configuration file because it adds complexity to testing (a test configuration file would become required to run the tests). Alternatively you can simply update the require 'cinch/plugins/word_game.rb' I put in lib/ayumi.rb and put it directly in the test that tests the associated class, but that feels pretty hackish to me.

bazz1tv commented 8 years ago

I've run it over my head for 15 minutes this is my conclusion.

I am against your strategy to "append" specific plugin loads to core bot files, since they are designed to exist and run independently of any plugins.

Perhaps I/we could make a utility / DSL

plugin add plugin create plugin remove

{action} the appropriate plugin-file, test file, and "load" code (wherever it is finally decided to go), and config stanza (depicting required settings and perhaps optional ones). This at least provides a clean interface to work through, and intended to report its activity like rails generate

Alternatively you can simply update the require 'cinch/plugins/word_game.rb' I put in lib/ayumi.rb and put it directly in the test that tests the associated class, but that feels pretty hackish to me.

The reason why I like this alternative is because it keeps the plugin code completely to itself, independent of the bot code. As long is there is a consistent standard location, this can easily be implemented. Custom locations would require additional thought.

Plugins are interesting because they should not be seen as "Ayumi" herself, but they directly need Ayumi to function, and they must be included by Ayumi. They must be easily added/removed to/from Ayumi.

One solution would be to simply require all the files (*) in the plugins folder. There are probably several reasons to be against that, but here's mine: I like the current config-file depiction, where config-presence of a plugin == actual-presence of the plugin. I like that visibility. And I figure the above solution can work successfully.


I'm going to file new issues for the following inspirations gathered from writing this comment:

bazz1tv commented 8 years ago

Be sure to read the previous comment on Github, as I made several edits which won't be visible from your email.