Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve logging #172

Closed
wants to merge 7 commits into from
Closed

Improve logging #172

wants to merge 7 commits into from

Conversation

gregemax
Copy link

flexible logging utility that adapts to both browser and CLI environments, ensuring consistent logging functionality across platforms.

@d-roak
Copy link
Contributor

d-roak commented Sep 24, 2024

Please read through all the comments and considerations i made in the past. This logger doesn't allow you to log objects at all

If you have questions about the intention behind the feature or need further clarification, please feel free to ask directly in our community group chats or here. I'm more than happy to give you extra clarifications

@gregemax
Copy link
Author

Hi @d-roak ,

Apologies for the confusion on my last PR. I understand the logger doesn’t fully meet the requirements, especially with object logging.

Could you clarify how the logger should handle object logging (e.g., deep nesting, structured key-value pairs)? Should it behave differently between CLI and browser environments? Are there any preferred libraries or patterns for logging objects? Additionally, could you provide an example of the expected output for logging objects or errors?

If there are any other aspects I should consider or add, please let me know. Thanks for your patience and guidance!

@d-roak
Copy link
Contributor

d-roak commented Sep 24, 2024

Object expansion only applies to the browser. You can think of it like console.log() does. E.g.
image

I don't remember any other consideration apart of all the ones mentioned in the issue and in ur last pr review. Apart of that, the pr is supposed to replace all console.log and console.err that currently exist in the code base.

Ideally we wanted to go with ts-log, but there were some issues with object logging (fullstack-build/tslog#295)

I didn't went through your code this time, i just pulled and tested it. Only after checking that it's working as intended, I'll look into the code

@gregemax gregemax closed this Sep 24, 2024
@gregemax gregemax mentioned this pull request Sep 24, 2024
@d-roak d-roak mentioned this pull request Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants