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

Plugin logger #4

Merged
merged 8 commits into from
Jun 26, 2024
Merged

Conversation

Keyrxng
Copy link
Member

@Keyrxng Keyrxng commented Jun 13, 2024

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
Copy link
Member Author

Keyrxng commented Jun 13, 2024

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
Copy link
Member

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

@Keyrxng
Copy link
Member Author

Keyrxng commented Jun 19, 2024

@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
Copy link
Member

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
Copy link
Member Author

Keyrxng commented Jun 20, 2024

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

@gentlementlegen
Copy link
Member

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

This was referenced Jun 22, 2024
@gentlementlegen gentlementlegen merged commit 0c4bdf8 into ubiquity-os:development Jun 26, 2024
1 check passed
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.

Decouple GitHub comments
2 participants