FuelRats / pipsqueak

ED Fuel rats IRC bot
Other
13 stars 14 forks source link

[BREAKING CHANGE]Renamed sopel-modules and the modules to abide by conventions. #254

Closed theunkn0wn1 closed 6 years ago

theunkn0wn1 commented 6 years ago

The modules unfortunately were named with - characters in their name. The unfortunate effect of this is that python will not recognize them, therefore they cannot be imported in other modules/scripts.

This is important if we want to do unit testing, or share resources across sopel modules.

Due to the fact the sopel-modules folder and the modules themselves have been renamed, the sopel.cfg file will need to be updated otherwise it will be unable to find the modules that make mecha squeak. (pun intended)

Marenthyu commented 6 years ago

Where exactly does this block anything? From how i understand it, modules are not supposed to be imported and in any case, sopel has to be doing it somehow behind the scenes themselves. Please elaborate more; but the name change shouldn't be a big problem otherwise.

But do be careful with importing modules, even if we go through with the change.

theunkn0wn1 commented 6 years ago

Basicially if this PR stays open too long im concerned that further changes may cause conflicts. Im not sure what would happen if something else was commited to develop, as ive had bad experiences with conflict resolution

Why its being imported, because Unit test files, from my understanding should not be in the same file as the module they test. Therefore they must be able to import the tested module. you cannot do

import my-module

as - cannot be used in a module name (at least during import, it results in a syntax error) which is precicely why the files are being renamed.

theunkn0wn1 commented 6 years ago

merge conflict... case in point

theunkn0wn1 commented 6 years ago

Alright, i only intended to fix the merge conflict, not merge it yet without your final approval maren. Doesn't appear anything broke when this got merged, you will just have to update your sopel.cfg when you update mechasqueak-test. Il leave the branch open for the moment should you decide to revert this