Implement a logger instead of print statements

heyjaywilson commented 2 weeks ago

Change all print statements to use Logger from OSLog.

[ ] Each package should have it's own log category
[ ] The logger's subsystem should be the product bundle identifier. If you need a default value use dev.cctplus.peakfit
[ ] The logger should live in the utilities package
[ ] The logger should be callable using PFLogger()
[ ] This PR should be labeled as a chore

Resources:

Modern logging with OSLog by Donny Wals

rajhraval commented 1 week ago

I would like to this issue.

heyjaywilson commented 1 week ago

@rajhraval it's all yours!

rajhraval commented 1 week ago

Just have some doubts:

Each package should have it's own log category

So it should be called like PFLogger.error(package: .design, error) Denoting it is an error from Design Package?

The logger should live in the utilities package

But if all packages import the Utilities package isn't it bad? That you are importing a single package too many times in different packages?

heyjaywilson commented 6 days ago

PFLogger.error(package: .design, error) I've tried doing stuff like this before, but I end up loosing functionality from the Logger in OSLog. I more so imagined it working like this:

// Error example
PFLogger.log(.design).error("message here")
// Debug example
PFLogger.log(.dataStorage).info("Saved a new exercise: \(exercise.name, privacy: .public)")

the .log function takes in a category that is whatever package it's being called from or even more specific category added later (like maybe one for a service) and returns a properly configured Logger with . The .error() is the Logger's built in error. This allows us to utilize the message capability to mark values as private if needed.

I should have probably put more of this in the issue description, so I'm sorry about that.

But if all packages import the Utilities package isn't it bad?

There shouldn't be an issue. This project uses a modularized code base and that's exactly what this does. If you have any docs or articles saying otherwise, then I'd love to read them.

Hopefully that helps answer some questions

rajhraval commented 6 days ago

Oh, like this. @heyjaywilson I have raised a PR with my kind of implementation not replaced with print statements yet. I will improve based on your opinion.

heyjaywilson commented 2 days ago

@rajhraval i'm gonna look at the PR after work today. Sorry for the delay.

heyjaywilson / peakfit

Implement a logger instead of print statements #19