Smokedetector: Refactor effort?

[Cerbrus]

The project lacks some structure. For example, findspam.py is a mix of rules, checking functionality, and finally, a method to check a post and generate a "why".

I've created a branch to see if there's something we can do to make the project more maintainable, and re-organized a few files.
The change moves 3 blacklist text files to a "blacklists" folder, and moves the "rules" and the "checking" functions to separate files.

Now I'm sure there's more that can be done, and I wouldn't be surprised if that change, as is, breaks SD (Seeing as I have 0 experience with Python), but I'm curious to see if the team is open to changes in the project's structure, like this.

[Cerbrus]

One issue is the auto blacklist: since the blacklist is moved, the path for this feature would have to be updated.

[ArtOfCode-]

In theory? Yeah, I'm totally open to refactors that make our code better. It's what, most of a year since we fixed any technical debt?

In practice, large refactors have a nasty habit of badly breaking Smokey. That's not a reason not to do it, but if we're going to, maybe we need to be more careful on this than we did on the last one.

[Vogel612]

the blacklist save-location issue is one that could be alleviated by loading the blacklist from configuration, and just "injecting" the reference in the autoblacklist command instead of hardcoding the path.

[dorukayhan]

large refactors have a nasty habit of badly breaking Smokey

What if we break down these large refactors into whole bunches of little organizational tweaks and then apply those tweaks incrementally?

[SmokeDetector]

That'd be the way to do it, yes. Do one thing at a time and do it carefully.

On Tue, Feb 21, 2017 at 1:59 PM dorukayhan [e-mail] wrote:

large refactors have a nasty habit of badly breaking Smokey

What if we break down these large refactors into whole bunches of little
organizational tweaks and then apply those tweaks incrementally?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/Charcoal-SE/SmokeDetector/issues/537#issuecomment-281478459,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKjSMxhRi0lu4ia-U6F9U1Bt_uioFCqdks5re1BIgaJpZM4MHQY6
.

[Cerbrus]

One step at a time makes sense, yea.

I'm guessing my attempt here could be a start, @dorukayhan?

Although maybe some of the folder / module structure could be named better. I'm sure there are rules I'm not aware of ;-)

[rschrieken]

I'm forced to ask this: does the public interface of the code you touch have a working unit test with reasonable coverage? If not, fix that first ...

[ArtOfCode-]

@rschrieken whaddaya mean, public interface? You think Smokey is well-structured enough that it has any public interface outside of chat? :P