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.

Sign up for GitHub

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

Jump to bottom

Feat: Custom logger #371

Merged
merged 7 commits into from
Mar 14, 2024
Merged

Feat: Custom logger #371

merged 7 commits into from
Mar 14, 2024

Conversation

RobinTail
Copy link
Contributor

I'm using express-fileupload for an application having more advanced loggers than Console.
I'd like to suggest the feature that enables consumers to configure the logger while preserving the console by default.

@coveralls
Copy link

coveralls commented Mar 1, 2024 •
edited
Loading

Coverage Status

coverage: 94.362%. remained the same
when pulling cb317f7 on RobinTail:feat-custom-logger
into 98028e9 on richardgirges:master.

@RobinTail RobinTail marked this pull request as ready for review March 1, 2024 23:47
@RobinTail
Copy link
Contributor Author

@richardgirges , please review

@RomanBurunkov
Copy link
Collaborator

Hi @RobinTail

Thank u for this PR.
Just one suggestion:
What if you add console to the defaults, and remove OR in logging function.
If it's ok, I will be happy to merge and publish it.

@RobinTail
Copy link
Contributor Author

I can do that, but it will also require me to update some existing tests of debugLog() utility, @RomanBurunkov
I will let you know once it's done.

@RobinTail
Copy link
Contributor Author

Done, @RomanBurunkov .
I made logger option to be console by default and removed OR from debugLog().
Instead, I added an additional check for the presence of the logger property, because of the tests that call that method directly. Those I updated as well.

@RobinTail
Copy link
Contributor Author

@RomanBurunkov , please review again.

@RomanBurunkov RomanBurunkov merged commit 4313856 into richardgirges:master Mar 14, 2024
7 checks passed
@RobinTail
Copy link
Contributor Author

RobinTail commented Mar 14, 2024 •
edited
Loading

Thank you @RomanBurunkov .
Could you release it as 1.5.0 so I could make another PR to @types/express-fileupload for describing these changes?

@RobinTail RobinTail mentioned this pull request Mar 16, 2024
Merged
8 tasks
@RobinTail
Copy link
Contributor Author

Types updating PR: DefinitelyTyped/DefinitelyTyped#69016
FYI, @RomanBurunkov

This was referenced Mar 16, 2024
Merged
Merged
RobinTail added a commit to RobinTail/express-zod-api that referenced this pull request Mar 16, 2024
@RobinTail
Feat: Using the configured logger for debugging uploads (#1623)
b0e8bdf 
I made the logger in `express-fileupload` configurable:
richardgirges/express-fileupload#371

So now the configured logger can be used for debugging the upload
instead of console.
That, however, also requires the types updated in a separate repo
(waiting for that):
DefinitelyTyped/DefinitelyTyped#69016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RobinTail @coveralls @RomanBurunkov