MasterJ93 / ATProtoKit

A straightforward solution for using AT Protocol and Bluesky, written in Swift.
https://atprotokit.cjrriley.com/documentation/atprotokit/
MIT License
40 stars 8 forks source link

Logging implementation for mapping on Apple and Non-Apple OSs #21

Open stoicswe opened 5 months ago

stoicswe commented 5 months ago

Description

Attempt for a logging implementation that maps to either the standard Apple logger if building for an Apple-OS target or the SwiftLogger library if the target is a non-Apple OS

Linked Issues

18

Type of Change

Checklist:

Screenshots (if applicable)

Attach any screenshots or GIFs showcasing the changes effect.

Additional Notes

Add any other notes about the Pull Request here.

Credits

If you want to be credited in the CONTRIBUTORS file, you can fill out the form below. Please don't remove the square brackets.

MasterJ93 commented 5 months ago

Thanks for your help with this. I took a quick look at it and there seems to be some misunderstanding with respect to this.

First, as of right now, the issue has a "draft" tag, which means that it's still being worked on before others can consider tackling it. However, I'll leave this pull request in here. Once that tag goes away, people can begin to solve the issues.

Second, as stated in the Contributing Guidelines, you need to put your pull requests in the develop branch, not main. main is like a "beta" version of ATProtoKit, while the releases are the official versions, and develop is the "alpha." I want develop to deal with the bugs and ironing them out. I want main to add features one at a time before being put into the releases (unless it's a bug fix, which in this case, it needs to be added straight into main). It seems that this file isn't visible enough and so I'll work toward making that possible even more.

Third, and finally, the current file you've edited, Logging.swift, wasn't something I was considering when making the draft. The purpose of the draft was to add log events into the various files, since this was something the project has been lacking on. However, I wasn't able to find time to do this, which is why I made it. That said, I would encourage you to keep working on it, because if there are indeed improvements to the file, I'll take it into consideration. That said, I'll soon update the issue to mention this.

stoicswe commented 5 months ago

Basic logging messages have been added to the classes across the library. The commits have been separated by a per-file basis to try to make the PR more reviewable. Please let me know if there are more areas to improve the logs.

MasterJ93 commented 5 months ago

Anything other than ATProtoKit, ATProtoTools, and ATProtoAdmin classes should not be given log events yet, as they're still in development and I feel there will be massive changes in there, still. However, I forgot about adding ATProtocolConfiguration and ATFacetParser, which are both complete, so thanks for making those changes in there.

Other than that, this is looking good, I think! 👍

stoicswe commented 4 months ago

Quick update on this PR, I have been busy the past week or so, but plan to get back to editing this likely tomorrow. Sorry about the delay to looking into this PR and getting the implementation completed.