Skip to content

Update metrics center manually to latest version#129788

Closed
drewroengoogle wants to merge 13 commits intoflutter:masterfrom
drewroengoogle:update-metrics-center
Closed

Update metrics center manually to latest version#129788
drewroengoogle wants to merge 13 commits intoflutter:masterfrom
drewroengoogle:update-metrics-center

Conversation

@drewroengoogle
Copy link
Contributor

@drewroengoogle drewroengoogle commented Jun 29, 2023

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@drewroengoogle drewroengoogle requested a review from keyonghan June 29, 2023 20:02
keyonghan
keyonghan approved these changes Jun 29, 2023
View reviewed changes
Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added a: internationalization Supporting other languages or locales. (aka i18n) d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos documentation f: integration_test The flutter/packages/integration_test plugin labels Jun 29, 2023
@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. framework flutter/packages/flutter repository. See also f: labels. labels Jun 30, 2023
christopherfujino
christopherfujino approved these changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@aam aam mentioned this pull request Jul 5, 2023
Closed
@zanderso zanderso mentioned this pull request Jul 5, 2023
Closed
8 tasks
@christopherfujino
Copy link
Contributor

christopherfujino commented Jul 5, 2023

Oh, looks like a regression actually:

Null check operator used on a null value
#0      _ProgramInfoBuilder.createInfoNodeFor (package:vm_snapshot_analysis/v8_profile.dart:443:53)
#1      _ProgramInfoBuilder.getInfoNodeFor (package:vm_snapshot_analysis/v8_profile.dart:374:14)
#2      _ProgramInfoBuilder.createInfoNodeFor (package:vm_snapshot_analysis/v8_profile.dart:439:21)
#3      _ProgramInfoBuilder.getInfoNodeFor (package:vm_snapshot_analysis/v8_profile.dart:374:14)
#4      _ProgramInfoBuilder.createInfoNodeFor (package:vm_snapshot_analysis/v8_profile.dart:420:18)
#5      _ProgramInfoBuilder.getInfoNodeFor (package:vm_snapshot_analysis/v8_profile.dart:374:14)
#6      ListIterable.forEach (dart:_internal/iterable.dart:39:13)
#7      _ProgramInfoBuilder.build (package:vm_snapshot_analysis/v8_profile.dart:338:16)
#8      toProgramInfo (package:vm_snapshot_analysis/v8_profile.dart:282:8)
#9      _treemapFromSnapshot (package:vm_snapshot_analysis/treemap.dart:158:27)
#10     treemapFromJson (package:vm_snapshot_analysis/treemap.dart:85:5)
#11     SizeAnalyzer.analyzeZipSizeAndAotSnapshot (package:flutter_tools/src/base/analyze_size.dart:123:59)
#12     AndroidGradleBuilder._performCodeSizeAnalysis (package:flutter_tools/src/android/gradle.dart:623:60)
#13     AndroidGradleBuilder.buildGradleApp (package:flutter_tools/src/android/gradle.dart:604:15)
<asynchronous suspension>
#14     AndroidGradleBuilder.buildApk (package:flutter_tools/src/android/gradle.dart:242:5)
<asynchronous suspension>
#15     BuildApkCommand.runCommand (package:flutter_tools/src/commands/build_apk.dart:116:5)
<asynchronous suspension>
#16     FlutterCommand.run.<anonymous closure> (package:flutter_tools/src/runner/flutter_command.dart:1297:27)
<asynchronous suspension>
#17     AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:150:19)
<asynchronous suspension>
#18     CommandRunner.runCommand (package:args/command_runner.dart:212:13)
<asynchronous suspension>
#19     FlutterCommandRunner.runCommand.<anonymous closure> (package:flutter_tools/src/runner/flutter_command_runner.dart:339:9)
<asynchronous suspension>
#20     AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:150:19)
<asynchronous suspension>
#21     FlutterCommandRunner.runCommand (package:flutter_tools/src/runner/flutter_command_runner.dart:285:5)
<asynchronous suspension>
#22     run.<anonymous closure>.<anonymous closure> (package:flutter_tools/runner.dart:115:9)
<asynchronous suspension>
#23     AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:150:19)
<asynchronous suspension>
#24     main (package:flutter_tools/executable.dart:90:3)
<asynchronous suspension>

It's not obvious to me what the regression is however, as the null check from the stacktrace has existed since dart-lang/sdk@458be23#diff-3d9d18113b83b258e333b124cdfd6c7c110fbb487c93c0185fb27ca8950ec1d9R445 in 2021. @sstrickl do you have context?

@christopherfujino
Copy link
Contributor

christopherfujino commented Jul 5, 2023

Oh wait, this is #130009. Apparently version 0.7.4 did NOT actually fix this?

@drewroengoogle drewroengoogle mentioned this pull request Jul 5, 2023
Closed
@drewroengoogle drewroengoogle mentioned this pull request Jul 6, 2023
Closed
@sstrickl
Copy link
Contributor

sstrickl commented Jul 7, 2023

Okay, version 0.7.6 of package:vm_snapshot_analysis should remove the issues on that side of things.

@christopherfujino
Copy link
Contributor

christopherfujino commented Jul 7, 2023

Okay, version 0.7.6 of package:vm_snapshot_analysis should remove the issues on that side of things.

Thanks, closing this PR in favor of https://github.com/flutter/flutter/pull/130163/files#diff-a3d4dfa7e9543c501bc1aee57d5502b657d6f5469c09fb42b8c9d516ec53df66R37

@drewroengoogle drewroengoogle deleted the update-metrics-center branch December 13, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@christopherfujino christopherfujino christopherfujino approved these changes

@keyonghan keyonghan keyonghan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: internationalization Supporting other languages or locales. (aka i18n) a: tests "flutter test", flutter_test, or one of our tests c: contributor-productivity Team-specific productivity, code health, technical debt. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: integration_test The flutter/packages/integration_test plugin framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@drewroengoogle @christopherfujino @sstrickl @keyonghan @Hixie