Skip to content

Fixes SlackWriter to evaluate the IO.#640

Merged
darkfrog26 merged 2 commits intooutr:masterfrom
KristianAN:master
Oct 19, 2024
Merged

Fixes SlackWriter to evaluate the IO.#640
darkfrog26 merged 2 commits intooutr:masterfrom
KristianAN:master

Conversation

@KristianAN
Copy link

This uses unsafeRunAndForget. This is not ideal, but currently there is no way to wire up an effectfull writer as far as I can tell. An effectfull writer would also need to be wired up in the application (maybe some would prefer this).

This uses unsafeRunAndForget. This is not ideal, but currently there is
no way to wire up an effectfull writer as far as I can tell. An effectfull writer
would also need to be wired up in the application (maybe some would
prefer this).

override def write(record: LogRecord, output: LogOutput, outputFormat: OutputFormat): Unit = {
val io = log(record)
val io = log(record) // Does nothing
Copy link
Author

Choose a reason for hiding this comment

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

I don't think this actually does anything here. I'm guessing it used to work because it was a Future that would evaluate anyway. Then when rewritten to IO it is just not evaluated.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're correct, this can be removed.

Move import to top level of file
@KristianAN KristianAN changed the title Fixes SlackWriter to actually evaluate the IO. Fixes SlackWriter to evaluate the IO. Oct 17, 2024

override def write(record: LogRecord, output: LogOutput, outputFormat: OutputFormat): Unit = {
val io = log(record)
val io = log(record) // Does nothing
Copy link
Contributor

Choose a reason for hiding this comment

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

You're correct, this can be removed.

@darkfrog26 darkfrog26 merged commit 5b56d4d into outr:master Oct 19, 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