Spring clean

[rakiru]

Eh, late Summer clean? Or more likely, Autumn to Winter clean. There's a lot of crap in utils that just got shoved there at the beginning without any real thought behind it. It may be worth having a scan through the rest of the code for less-than-stellar code (instance variables set to mutable type in class definition, for example (or even just instance variables referenced in class defs at all)). Perhaps Landscape might be useful for once.

Edit: Some possible changes too:

• Rename command_manager to commands
  • Reasoning: command_manager.CommandManager is a bit redundant, and any other command-related stuff would probably go in there too. This could also be a full module rather than a single file.
• Rename event_manager to events
  • Reasoning: Same as commands
• Deprecate system.plugin
  Reasoning: It's just an import of system.plugins.plugin.PluginObject. We have a function decorator for deprecation, so we should make a class one and wrap this.
• Rename Manager in factory_manager to FactoryManager
  • Reasoning: Overly generic name means you need the full module name to make any sense of it.
• Move functionality of run.py into an Ultros class.
  • Reasoning: Cleaner, separates instantiation (args parsing, etc.) from bot setup and running. This would also be a sensible place to store/access references to subsystems were we to get rid of the singletons.
• Discuss system.help
  • Reasoning: What even is this? We should probably discuss commands a bit to figure out how they would provide help topics and such.
• Plugin cleanup
  • Different repo, but it's a cleanup thing. A bunch of plugins still import and grab instances to things such as the command and event manager. The plugin superclass has instances of these already now, so this unnecessary code should be stripped out of existing plugins.

Any renaming should be deprecated. Also, we should think up how to handle deprecation (when do we actually remove things?).

Other things to consider but should be discussed in separate tickets if/when tackled:

• Rethink overzealous use of singletons.
• Rethink Protocol reuse (same Protocol instance reused for every connection).
  • It didn't actually reuse them, but there was still some weird bullshit system for connections. Fixed with Unfuck factories #90.
• Rethink translation system
  • Current one isn't very useful and underutilised. I think g said it was a bad system.
• Rethink versioning
  • Lots of breaking changes without even bumping patch version, making them pretty useless.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/26491528-spring-clean?utm_campaign=plugin&utm_content=tracker%2F269930&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F269930&utm_medium=issues&utm_source=github).

[gdude2002]

Commands: #49, #56

Rethink overzealous use of singletons.

I would be the first person to say that people complaining that Singletons are always an awful idea are stupid, but we do use a crapton of them and they aren't needed nearly as much as they used to be.

Rethink Protocol reuse (same Protocol instance reused for every connection).

We.. don't do this. Do we? I didn't think we did.

Rethink translation system

The main problem I had was that we already had 213987328746238732462 strings in Ultros and going through and replacing them with tokens manually is an extreme pain in the ass. Don't get me wrong - the system works - but I'd rather a Java-style properties approach than this stupid .po .mo crap, and some translation site like Crowdin.

Rethink versioning

Yeah, I'd be doing all this if I remembered to, but.. I don't. Halp!

[rakiru]

All the things you quoted were things that I meant for separate tickets and just put here so I didn't forget. :V
Does that mean you agree with all the checkboxed items? The last one should probably also be a separate ticket, but I stumbled across it while documenting the basic structure for CODING.md.

[singletons snip]

Off the top of my head, I can't think of a place where a singleton is needed. Perhaps the factory manager, but even then, you could arguably use multiple of those with different base config paths or something. They're restricting a lot of things to only one instance when there's really no need to do so, and the only reason it's been done that way is we haven't had any form of central "bot" to store references to the instances in, just a bunch of code that's been loaded and knows the rest exists by path. It also makes it impossible to switch any of this out after startup.

As for the "people complaining that Singletons are always an awful idea are stupid" argument, you could say that about anyone who sees anything in absolute black and white terms. The general reasoning behind it though is that they're so often used badly or when there's a better way to do it.

We.. don't do this. Do we? I didn't think we did.

system.factory.Factory().buildProtocol()

[translation snip]

I have no experience with translation, so I have no idea what approaches there are to it and what benefits/problems they have. I just know we've discussed the existing system we have and it doesn't really make sense for us. A separate ticket should be made for this with a list of requirements, perhaps?

Yeah, I'd be doing all this if I remembered to, but.. I don't. Halp!

Yeah, it's not something I ever remember either. Perhaps once we've discussed the issues in this ticket and got a more solid plan of what to do going forward, we can actually use the milestones feature and be a bit more liberal with the version bumps.

[gdude2002]

Does that mean you agree with all the checkboxed items?

Yis.

[Singletons]

Yeah, I agree with you, I picked singletons for ease of use to start with but moving them out is definitely not a bad idea.

system.factory.Factory().buildProtocol()

Yeah, but.. Factory() is a new instance?

[Translations]

Yep. Python uses .po and .mo files, which is the GNU way of doing things, but I just can't get behind it, it's too complicated.

[Versioning]

Yeah, I think so. The problem is remembering to do it - maybe we should add a git hook on commit to remind us with a popup or something.

[rakiru]

Yis.

Awesome, I'll get on that soon.

Yeah, but.. Factory() is a new instance?

Ah, I see the confusion. Two different IRC connections will obviously get their own Protocol instance, but if IRC instance 1 dies, instead of that factory making a new IRC instance to reconnect, it reuses the same one.

Yeah, I think so. The problem is remembering to do it - maybe we should add a git hook on commit to remind us with a popup or something.

I think if we just actually have a process for adding new features (issue to plan it, separate branch, PR), then versioning can just become a part of that. A git hook that goes "Hey, you've touched one of the core files but not versions.py - does it need bumped?" could be useful, but I think it will be wrong the vast majority of times, meaning it just gets ignored.

[gdude2002]

...instead of that factory making a new IRC instance to reconnect, it reuses the same one

Ah, I see. Well, I guess we could make a new one, yeah. Shouldn't be too hard.

[Versioning]

Yeah, makes sense. We'll have to come up with some standards and stick to them - Maybe decide on not ever pushing to master unless reviewed PR or minor fix?

[rakiru]

Shouldn't be too hard.

I'm pretty certain that's the default behaviour (and largely why I suggested it).

[Versioning]

Yeah, I was thinking something along those lines. I'm working on conventions branch again, so I'll make some provisional guidelines and we can discuss from there. I'll open a PR on it when the basics are done.

[gdude2002]

Sounds good - Will keep an eye out as I recover from this blasted viral infection

[gdude2002]

Additional stuff:

• Consider __main__.py
  • Could relate to making run.py importable
• Consider better API representation
  • For example, locations of various paths
• Consider making all paths configurable or specified in environment variables
  • I don't think this should be some crazy pathing system, but it's worth doing

[rakiru]

• Review usage of unicode/bytes. We're mostly fine, but occasionally it bites us. Sections of particular concern:
  • Web - file editor errors on UTF-8 files
  • Logging - see System logger isn't unicode-safe on Linux #71

[gdude2002]

Rename event_manager to events

We already have a module named events - perhaps use events.manager?