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

Override Notion Client logger #2620

Merged
merged 3 commits into from
Nov 22, 2023
Merged

Override Notion Client logger #2620

merged 3 commits into from
Nov 22, 2023

Conversation

@PopDaph PopDaph requested a review from fontanierh November 22, 2023 12:40
message: string,
extraInfo: Record<string, unknown>
) => {
console.log(`[${level}]: ${message}`, extraInfo);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not directly use a logger here? potentially deactivating after that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We def don't want those logs to clutter our DD errors no ? The'll handle the requests to notion, and we'll do the logging if where we need it wdyt ?

Copy link
Contributor

@spolu spolu Nov 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too worried about cluttering. But my point here is that console.log will clutter more than logger ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the console.log and no-op instead > want me to add back a log in info?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK added back with a logger.info so we keep them but stop being spammed with errors

@PopDaph PopDaph merged commit 2960ba3 into main Nov 22, 2023
2 checks passed
@PopDaph PopDaph deleted the override-notion-client-logger branch November 22, 2023 13:48
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