Skip to content

Conversation

jeswr
Copy link
Contributor

@jeswr jeswr commented Jan 20, 2023

This PR adds the feature of allowing users to pass a custom Logger to bashlib functions.

@jeswr
Copy link
Contributor Author

jeswr commented Jan 23, 2023

@Dexagod could you please review this when you get the chance (I lack the permissions to add you as a reviewer)

@mielvds mielvds requested a review from Dexagod January 23, 2023 08:43
@mielvds
Copy link
Collaborator

mielvds commented Jan 23, 2023

Thanks, looks good! Not a result of this PR, but I wonder whether we still need both the writeErrorString methods and the (logger || console).error constructs. It would be cleaner to use one method.

@Dexagod
Copy link
Collaborator

Dexagod commented Jan 23, 2023

I know I've been a tad inconsistent as well with my logging.
I'll take a look at making it a bit more consistent over the whole codebase.

@Dexagod Dexagod merged commit 1caa484 into SolidLabResearch:master Jan 23, 2023
@jeswr jeswr deleted the feat/custom-logger branch January 23, 2023 09:55
@jeswr
Copy link
Contributor Author

jeswr commented Jan 23, 2023

I'll take a look at making it a bit more consistent over the whole codebase.

@Dexagod - just a point to consider is that I am using this to throw an error when logger.error is called.

@jeswr
Copy link
Contributor Author

jeswr commented Jan 23, 2023

Thanks for merging this so fast btw; a release when you get the chance would be very useful :)

@Dexagod
Copy link
Collaborator

Dexagod commented Jan 23, 2023

I've updated the npm package so it should be usable with a global install from the CLI already.
A full release however will require at least a partial test coverage, and fixing session refresh.
Not sure when I'll get to that sadly :/

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.

3 participants