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

[flutter_tools] do not use context logger in gradle #75940

Merged
merged 2 commits into from
Feb 13, 2021

Conversation

jonahwilliams
Copy link
Member

Part 2 of 999 to clean up gradle.dart . move the logger into the class definition and remove context usage. Also removes sha analytics event that has not been useful (its always pretty fast 🤷🏻‍♂️ ).

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Feb 12, 2021
@google-cla google-cla bot added the cla: yes label Feb 12, 2021
final List<int> bytes = file.readAsBytesSync();
globals.printTrace('calculateSha: reading file took ${sw.elapsedMilliseconds}us');
Copy link
Member Author

Choose a reason for hiding this comment

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

I reviewed the data here and basically this is always super fast, the reported issue was likely unrelated

Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm w/ question

@@ -169,7 +169,7 @@ void createSettingsAarGradle(Directory androidDirectory) {
final String currentFileContent = currentSettingsFile.readAsStringSync();

final String newSettingsRelativeFile = globals.fs.path.relative(newSettingsFile.path);
final Status status = globals.logger.startProgress('✏️ Creating `$newSettingsRelativeFile`...');
final Status status = logger.startProgress('✏️ Creating `$newSettingsRelativeFile`...');
Copy link
Member

Choose a reason for hiding this comment

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

Could you check that ✏️ is in the list to rewrite when emoji aren't supported? I think it's base/console.dart or base/terminal.dart?

Copy link
Member Author

Choose a reason for hiding this comment

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

It wasn't. I added it to the list, and added tests for the rewriting since I couldn't find any

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

still lgtm

@jonahwilliams jonahwilliams merged commit dce8f71 into flutter:master Feb 13, 2021
@jonahwilliams jonahwilliams deleted the gradle_cleanup_2 branch February 13, 2021 01:38
@jonahwilliams
Copy link
Member Author

Part of #71511

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants