Skip to content

Detect whether the Debug Extension was built for dev or release #1800

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

Merged
merged 7 commits into from
Nov 29, 2022

Conversation

elliette
Copy link
Contributor

@elliette elliette commented Nov 29, 2022

Detects whether the Debug Extension was built for dev or release.

In dev mode:

  • only logs to the console in dev mode (unless specified otherwise)
  • uses the dev icon instead of the prod icon

This PR also moves the console.log implementation into logger and makes it private, so that the only way to log to the console is with the debugLog utilities.

@elliette elliette changed the title Detect whether or not the Debug Extension was built for dev or release Detect whether the Debug Extension was built for dev or release Nov 29, 2022
@elliette elliette requested a review from annagrin November 29, 2022 21:42
Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

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

LGTM with comments.

_newKeyValue(
oldLine: line,
newKey: 'name',
newValue: '[DEV] MV3 Dart Debug Extension',
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! That is really helpful to distinguish them from each other!

String msg, {
String? prefix,
bool devOnly = true,
}) {
_log(msg, prefix: prefix);
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to pass devOnly to _log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good catch!

String msg, {
String? prefix,
bool devOnly = true,
}) {
_log(msg, prefix: prefix, level: _LogLevel.warn);
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

debugError(
String msg, {
String? prefix,
bool devOnly = true,
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

_LogLevel? level,
String? prefix,
}) {
if (devOnly && !isDevMode()) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be reasonable to rename devOnly to verbose and maybe flip the default to false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sg, switched to verbose!

Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

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

LGTM!

@elliette elliette merged commit 1258510 into dart-lang:master Nov 29, 2022
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.

2 participants