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 structured logging feature #1938

Merged
merged 6 commits into from
Sep 20, 2023
Merged

Conversation

fmeriaux
Copy link
Contributor

@fmeriaux fmeriaux commented Aug 31, 2023

Close #1932

Before submitting pull request:

  • Check if the project compiles by running sbt compile
  • Verify docs compilation by running sbt compileDocs
  • Check if tests pass by running sbt test
  • Format code by running sbt scalafmt

@fmeriaux
Copy link
Contributor Author

@adamw I've tried something, but I'm not sure if I can add a unit test on this feature?

@adamw
Copy link
Member

adamw commented Sep 11, 2023

Sorry not sure how I missed this. Testing logging is difficult, but if you have an idea on how to approach this, go ahead :)

@fmeriaux
Copy link
Contributor Author

We should be able to test DefaultLog by making a SpyLogger that buffers what we send it.

I'm also going to separate the context construction into a separate class to test it in isolation and allow users to provide their own implementation of the context.

@fmeriaux fmeriaux marked this pull request as ready for review September 12, 2023 12:59
@fmeriaux
Copy link
Contributor Author

@adamw Can you give me a clue as to how to solve the CI failure? I'm having trouble understanding what's wrong.

@adamw
Copy link
Member

adamw commented Sep 16, 2023

It seems that once again the JS builds are broken because of problems with Chrome version / Selenium / some plugin. I'll take a look once I get some time :)

@adamw
Copy link
Member

adamw commented Sep 19, 2023

This looks great, thanks for your work! I fixed the JS build & left two minor comments on the code

@adamw adamw merged commit c1468c8 into softwaremill:master Sep 20, 2023
12 checks passed
@adamw
Copy link
Member

adamw commented Sep 20, 2023

Thanks for the fixes :)

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.

Add default structured logging ?
2 participants