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

Improve codegen warnings when missing deps #1975

Merged
merged 3 commits into from
Apr 11, 2024
Merged

Conversation

frandiox
Copy link
Contributor

When codegen deps are missing, this was shown before:

image

After these changes, we now show warnings:

image
@frandiox frandiox requested a review from a team April 11, 2024 14:52

This comment was marked as off-topic.

This comment was marked as off-topic.

Comment on lines -79 to +88
if (/\.body\[\d\]/) return;
if (/\.body\[\d\]/.test(message)) return;
if (/console\.time(End)?\(\)/.test(message)) return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a bug, the RE wasn't tested so it was always truthy

Copy link
Contributor

Choose a reason for hiding this comment

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

😱

Copy link
Contributor

Choose a reason for hiding this comment

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

What was the console.time added for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you run with --verbose you start seeing a loooot of warnings from Codegen that include .body or console.time(). I guess Codegen dependencies are also looking for a --verbose flag or similar, not sure.
Just muting these so that the output in the terminal is useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth a comment for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's already one above this diff: // Filter these logs even on verbose mode because it floods the terminal:

@frandiox frandiox merged commit 94a0c63 into main Apr 11, 2024
13 checks passed
@frandiox frandiox deleted the fd-codegen-warning branch April 11, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants