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 context to ApplicationLogPayload #293

Merged
merged 1 commit into from
Feb 6, 2023
Merged

Add context to ApplicationLogPayload #293

merged 1 commit into from
Feb 6, 2023

Conversation

bilfeldt
Copy link
Contributor

@bilfeldt bilfeldt commented Feb 3, 2023

The log context is not being passed to the ApplicationLogPayload class currently (relates to #162) and this PR simply passes that data.

Unfortunately it does not mean that the context is shown in Ray, because apparently the formatting of the context is not currently implemented in the Ray app.

Where is the formatting logic for the Payload subclasses? 🤔 is that somewhere in https://github.com/spatie/ray or the Electron app itself?

I propose that we show the message like we currently do, but that we add the context using the same formatter used for the Spatie\Ray\Payloads\CarbonPayload class or the \Spatie\Ray\Payloads\TablePayload

@freekmurze freekmurze merged commit 0c28a82 into spatie:main Feb 6, 2023
@freekmurze
Copy link
Member

Thanks!

Where is the formatting logic for the Payload subclasses? 🤔 is that somewhere in https://github.com/spatie/ray or the Electron app itself?

This is done within the Ray app itself.

@bilfeldt
Copy link
Contributor Author

bilfeldt commented Feb 6, 2023

Thanks!

Where is the formatting logic for the Payload subclasses? 🤔 is that somewhere in https://github.com/spatie/ray or the Electron app itself?

This is done within the Ray app itself.

@freekmurze is this something I can contribute to or is that a closed repo? Did not find it.

@freekmurze
Copy link
Member

That's a closed repo.

@bilfeldt bilfeldt deleted the patch-1 branch February 8, 2023 12:15
@bilfeldt
Copy link
Contributor Author

And the missing context in the Ray app is now fixed as well by @freekmurze closing the last part of this PR 🚀 https://twitter.com/freekmurze/status/1624073001854545920?s=46&t=lmI6pPi6EyTxQ1yroCFMFw

@freekmurze
Copy link
Member

Thanks for your work on this 👍

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