InfiniTime icon indicating copy to clipboard operation
InfiniTime copied to clipboard

Remove leftover debugging logs

Open Riksu9000 opened this issue 2 years ago • 2 comments

Remove NRF_LOG_INFO, as they're mostly leftovers from debugging. Devs can add the logs they need to their working copy during development instead of relying on the ones created by someone else.

Riksu9000 avatar Aug 13 '22 14:08 Riksu9000

If I'm not wrong, I think we should also be able to remove a lot of #include <nrf_log.h> together with those calls to NRF_LOG_INFO.

JF002 avatar Aug 21 '22 10:08 JF002

TL;DR Why would you remove logging? What's the problem this solves?

I'm a bit confused why these are removed. Logging is not a "leftover from debugging", but an essential tool for debugging. Consider logcat on Android being empty by default and if something is weird, you have to patch in logging yourself to have the slightest idea what is even going on.

One thing I really like about InfiniTime is how approachable it is for inexperienced programmers. There are several members of the community who got into embedded programming by playing around with InfiniTime, as anyone lurking the chat rooms for a while can attest to.

Compare the following two situations when someone new to the project reports weird behavior in the chat:

  1. "Hey, I tried to do X, but Y happens and I have no idea what's going on"
  2. "Hey, I tried to do X, but Y happens and when it does, there's always this message about Z in the log"

In the first case, experienced community members would have to first spend time helping the person figure out where adding log messages could be useful.

In the second case, there is a good chance the person wouldn't even ask in the chat, because the log message gives enough hints which parts of the code to look at for clues.

If the log is too noisy, logging could be cleaned up, e.g. by changing the log level from info to warning, as this PR does in the SystemMonitor. I like that, since it would allow filtering out all info messages, while retaining them for cases where verbose logging can be useful.

If there's a reason for removing all logging, please elaborate a bit, so others can at least understand the decision. Until then, I'd strongly oppose merging this PR.

ght avatar Sep 15 '22 23:09 ght