trackma icon indicating copy to clipboard operation
trackma copied to clipboard

Simplify messenger calls

Open imsamuka opened this issue 1 year ago • 4 comments

I changed the Messenger class to hold a internal class name. Every other class that uses the messenger (Engine, Data, Lib and TrackerBase), will instance the messenger class with their self.name only once, and every call to this instance, will optionally re-use the given name.

I say optionally, because debug, info, warn and exception can still receive a class name, so Hooks continue exactly the same: that's not a breaking change.

I searched and removed self.name from every debug, info, warn and exception call in the code base. As a bonus, almost every two-line call was inlined.

-     self.msg.warn(
-         self.name, "Unknown operation in queue (%s), skipping..." % repr(operation))
+     self.msg.warn("Unknown operation in queue (%s), skipping..." % repr(operation))

The code is now simpler, and self.name is only used twice in each of those classes.

I revised the changes thrice, but feel free review yourself :smiley:

imsamuka avatar Aug 07 '22 15:08 imsamuka

I thought the same regarding the logging module, i feel like we are reinventing the wheel here. But well, its working :+1:.

I changed the function signatures, now it's more obvious. I could make type annotations too, but the parameters explain themselves.

def debug(self, msg, *, name=None):

Since it is uncommon, the name can only be passed as a kword. It will give us more flexibility changing the methods in the future. If you need to use the same name multiple times, you should use the with_classname messenger method before anyway.

imsamuka avatar Aug 07 '22 22:08 imsamuka

I think breaking the hook API is fine; but I'd prefer to break it only once. If we break it this time, and we later move to the logging module, we'll break it twice. While I appreciate your work, I think if we plan to move to the logging module we should do that now instead.

z411 avatar Aug 09 '22 20:08 z411

I see, so i will revert the last commit. The function signatures will become harder to follow, but the API won't break right now, and we can change to the logging module with more ease.

imsamuka avatar Aug 09 '22 21:08 imsamuka

Done. The hooks continue the same :+1:

imsamuka avatar Aug 09 '22 21:08 imsamuka

Thank you!

z411 avatar Aug 15 '22 18:08 z411