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

Add logging to file functionality #444

Closed
wants to merge 16 commits into from

Conversation

JackCrumpLeys
Copy link
Contributor

Log to a /logs directory and compress old logs. #90

@JackCrumpLeys
Copy link
Contributor Author

I think some more discussion may be needed around log filtering and weather we want a really verbose log for crashes. I have tried to make it easy to just subscribe new writers onto the tracing log. It is a little bit against the overall layout of the code-base to put so much code into the main.rs, do we need to move it?

Copy link
Collaborator

@Indy2222 Indy2222 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am happy to see this effort. Please let me know if you need any help or want to discuss anything on Discord.

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
crates/log/src/setup.rs Outdated Show resolved Hide resolved
@Indy2222
Copy link
Collaborator

Indy2222 commented May 6, 2023

I think some more discussion may be needed around log filtering and weather we want a really verbose log for crashes. I have tried to make it easy to just subscribe new writers onto the tracing log. It is a little bit against the overall layout of the code-base to put so much code into the main.rs, do we need to move it?

Yes, I would move most of the logging setup code to de_log crate.

@JackCrumpLeys
Copy link
Contributor Author

JackCrumpLeys commented May 7, 2023

I think I have resolved all of those issues, my fork will run the actions I'll wait on those but I think its all good.
de_log probably does not need to be a proper plugin just be passed app so it can insert the guard. For the future I do think that it would be good to have a very verbose log just for crash stuff and then a normal verbosity log for general things. That is why I have used the layer pattern to build the logger because using layers you can add writers with different directives for more complicated logging in the future.

EDIT: The actions passed (except for pushing to gh pages)

any further comments?

crates/log/src/setup.rs Outdated Show resolved Hide resolved
crates/log/src/setup.rs Outdated Show resolved Hide resolved
crates/log/src/setup.rs Outdated Show resolved Hide resolved
@Indy2222
Copy link
Collaborator

Indy2222 commented May 9, 2023

Superseded by #447 (I could not push to this branch).

@Indy2222 Indy2222 closed this May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants