Implement log system

[OJarrisonn]

This PR ain't complete so DO NOT MERGE IT YET, this is just to discuss around the current status of this feature.

• Introduces a new dependency: chrono
• Introduces a new configuration option: log_path
• Adds a Logger singleton that is non-thread safe
• Known issue: the user must manually create log_path folder before using
• Known issue: memory utilization: log entries are stored both in memory and in disk
• No logging level (INFO, WARN, FAIL), etc

This PR also attends to the new need to document code, so most of the new features are well-documented

Closes: #36

[davidbtadokoro]

Hi @OJarrisonn, thanks a lot for this PR! I will take a closer look at it to begin discussions ASAP.

[davidbtadokoro]

Hi @OJarrisonn, and thank you so much for the update! Also, marvelous work on the logging levels :+1:

I've made changes to your PR, which you can check at this branch. Note that the commit messages contain a Maintainer edits section that overviews my modifications.

If you agree with this proposal, we can move to merging!

[OJarrisonn]

I just disagree with the changes in the fields names for Logger, i rather use shorter names like: file, filepath, logs and level

Also, was the log level filtering logic really inverted? level <= self.print_level means that if we set the filter level to Warn we will only print Info and Warn logs, and discard Error. Is this really the expected behaviour?

[davidbtadokoro]

I just disagree with the changes in the fields names for Logger, i rather use shorter names like: file, filepath, logs and level

I agree that whenever possible, we should favor shorter names. By now, you've gathered that I am really verbose in names :sweat_smile: , but at this time, I think logs and level is too ambiguous. Imagine someone who first sees the code; the name logs would prompt him to think that this vector will amass all logs, while this isn't true; also, level doesn't convey its means.

Also, was the log level filtering logic really inverted? level <= self.print_level means that if we set the filter level to Warn we will only print Info and Warn logs, and discard Error. Is this really the expected behaviour?

Unfortunately, it is indeed inverted... Try adding a warn() and an error() right below the info("patch-hub started") and changing the print level of the Logger to Warning. Notice that the warning and the error will be printed when the warning and info should be printed instead.

Simulating the code, if the precedence is Info < Warning < Error, if self.print_level == Warning, the check

if self.print_level <= Info

will be false, making this inversion...

UPDATE:

Reading again the second question, I understood that you do understand the behavior; you just don't agree with it.

IMHO, if the level set is Warning, we should notice Info and Warning, as the higher the precedence, the more critical the log. In other words, there is a justification that if the user wants to see critical errors, it follows that he/she also wants to see the usual info.

However, I get the "compiler-logic", in which the less critical the log is, the more noisy it is. So errors should always be thrown, while warnings should not always be thrown, and so on. Note, however, that patch-hub is a user tool, so the warnings and errors from the context of compilers are different from the ones in the project scope.

To summarize, I am genuinely split between the two approaches...

[OJarrisonn]

Yeah, my approach is to print logs that are more severe than the filter level. We should always print the errors, while Info aint always relevant to the common user. Error messages always are relevant so fhe user might try to fix it by himself

Remember that the users of patch-hub are kernel developers, so we must provide the kind of logging that a developer might expect

In your approach we will always see the "patch-hub started" message when closing the app. Is this messeage really that important that we always print it to the terminal?

[davidbtadokoro]

Yeah, my approach is to print logs that are more severe than the filter level. We should always print the errors, while Info aint always relevant to the common user. Error messages always are relevant so fhe user might try to fix it by himself

Remember that the users of patch-hub are kernel developers, so we must provide the kind of logging that a developer might expect

In your approach we will always see the "patch-hub started" message when closing the app. Is this messeage really that important that we always print it to the terminal?

True! After mulling over the matter, I totally agree with the approach I called "inverted". To be honest, now I don't see why I thought the other approach was better 🥲

Thanks for sticking with your opinion. I could've made the wrong decision, otherwise.

[davidbtadokoro]

@OJarrisonn , considering the precedence problem fixed, are we cool to merge this change, on your end?

[OJarrisonn]

Yes

[davidbtadokoro]

Nice, going to do this soon.

Just as a head up, I might still have to solve some merge conflicts due to the last merged change. This probably won't change anything "important" or that we discussed, but just to keep the process transparent.

[davidbtadokoro]

Hey @OJarrisonn, just merged the change into the unstable branch :+1:

Thank you so much for your continuous effort and great work! The logging feature looks really neat and will be crucial for users and developers alike :fire: