ubiquity-os / ubiquity-os-logger

0 stars 6 forks source link

Plugin logger #4

Closed Keyrxng closed 5 months ago

Keyrxng commented 5 months ago

Resolves #2

Because indeed they could clutter the main db with their own logs.

It makes sense to remove the direct capability to do this from within the logger itself.

have nice formatting for comments easily

formatting and styling remains the same, when the logger calls _log it returns a LogReturn object which the invoker can use to post or store.

I don't think it is a good idea to tie Supabase nor GitHub into it as we don't necessarily want to post or save to database, but rather simply log stuff.

I have removed the Supabase and GH comment posting ability completely (can be returned if the need for it exists tho it seems more likely only the kernel should be able to post to SB and comment control is on a per-plugin basis and this should be a simple runtime logger and nothing more)

Keyrxng commented 5 months ago

Because this needs to run in a worker env it's likely util will need replaced with an alternative. Here I just went with JSON.stringify(), I did try the npm package util but didn't have much success with it.

@gitcoindev what would your suggestion be here if it does need replaced?

const messageFormatted = typeof message === "string" 
? message 
: util.inspect(message, { showHidden: true, depth: null, breakLength: Infinity });
gentlementlegen commented 5 months ago

@Keyrxng The dist folder got into the commits, I don't think that's wanted.

Keyrxng commented 5 months ago

@Keyrxng The dist folder got into the commits, I don't think that's wanted.

https://github.com/ubiquity/ubiquibot-logger/tree/development/dist

I was keeping things the same as the dev branch but if that is not wanted I shall remove it.

gentlementlegen commented 5 months ago

I think this was a mistake to commit it because it contains generated js and map which are most likely generating by bundling tools. You also might want to update the .gitignore because it only ignores static/dist.

Keyrxng commented 5 months ago

Agreed and I have included it in the .gitignore in the same commit I removed the dir

gentlementlegen commented 5 months ago

@Keyrxng Thank you. Is there any tests associated with the changes in this repo? If not, should definitely nice to have in another PR.