sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
949 stars 403 forks source link

Review `sopel.config` for dead/outdated code #2556

Open SnoopJ opened 10 months ago

SnoopJ commented 10 months ago

It was pointed out on IRC that sopel.Config.CoreSection uses a narrow type-check for lists rather than isinstance(), and somewhat surprisingly, it turns out that this code is not currently under coverage by the test suite. The code is 11 years old, so it's possibly vestigial in nature, but investigation into whether or not this should be considered a bug (i.e. whether one could do cfg.core.somefield = MyListSubtype(...)) turned into a broader discussion about dead code in the config machinery.

For whatever it's worth, testing with cfg.core.enable = ... confirms that subtypes of list are accepted, although sequence types that are coercible to list (in particular tuple) are not accepted. Debatable whether or not this is a feature or a bug, but the early view seems to be that this is fine for ListAttribute which is pretty specifically about lists.

This issue is a placeholder for the intent to sweep for old code in the config machinery. A very optimistic goal for closure would be to get coverage of the config machinery up to 100%, probably a combination of removing dead code and improved testing.