search

ubiquity-os / ubiquity-os-logger

0 stars 6 forks source link

Properly set the `metadata` value of `LogReturn` #46

Open gentlementlegen opened 4 weeks ago

gentlementlegen commented 4 weeks ago 
          Revision should point to the current version of the plugin. Within actions this is easy to get, within workers I don't think this is possible, instead we could probably use the version id?
image

_Originally posted by @gentlementlegen in https://github.com/ubiquity-os/ubiquity-os-kernel/pull/169#discussion_r1818077183_

imabutahersiddik commented 4 weeks ago

/start

ubiquity-os-beta[bot] commented 4 weeks ago
Deadline Mon, Oct 28, 6:27 PM UTC
Beneficiary 0x7245F5Cb278ea948Ab6302Bd911db00Ad4889672

[!TIP]

  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.
ubiquity-os-beta[bot] commented 3 weeks ago

Passed the deadline and no activity is detected, removing assignees: @imabutahersiddik.

Keyrxng commented 2 weeks ago

@gentlementlegen we gonna built it into the logger or keep it within the sdk?

gentlementlegen commented 2 weeks ago

@Keyrxng The SDK itself uses the logger but maybe having this metadata in the logger pollutes the code base too much as it is specific to GitHub comments probably.

Keyrxng commented 2 weeks ago

I thought something similar, so best to keep it all within the SDK in error metadata. Can close this or transfer it or whatever as it's resolved within the SDK following the merge of https://github.com/gentlementlegen/ubiquity-os-kernel/pull/1 (will PR the SDK repo now that there is one)

gentlementlegen commented 2 weeks ago

@Keyrxng should we reopen the PR then?

Keyrxng commented 2 weeks ago

I'm including all of the logic for this directly into the SDK right? If so then yeah I'll PR the SDK.

Just seen foo's comment re: SDK separation complicating things. Waiting to see if the SDK moves back into the kernel or stays separate

gentlementlegen commented 2 weeks ago

@Keyrxng Also goes for the PR you just closed I guess.

Keyrxng commented 2 weeks ago

@Keyrxng Also goes for the PR you just closed I guess.

Yes unfortunately, would have been ideal if you had managed to merge into your improvements PR like you suggested but it's all good I'll just re-do them.

I assume the SDK is 100% staying separate from the kernel then yeah and I should PR against it?

gentlementlegen commented 2 weeks ago

Let's see how we do it. If the circular reference can be fixed it can remain inside the kernel and it's fine.