-
Notifications
You must be signed in to change notification settings - Fork 100
Filtered Android tasks should include mokey and pixel 7 pro in dashboard #3902
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
Conversation
dashboard/lib/widgets/task_icon.dart
Outdated
| final String matchedName = qualifiedTask.task!.toLowerCase(); | ||
| final bool isWebTest = matchedName.contains('_web') || matchedName.contains('web_'); | ||
| final bool isToolTest = matchedName.contains('_tool') || matchedName.contains('tool_'); | ||
| final bool isAndroidTest = matchedName.contains('_android') || matchedName.contains('_mokey'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the instances of _android should be gone, and I think all we have now are _mokey and _pixel_7pro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah thanks, I forgot about pixel_7pro. I think keeping _android is pretty harmless, I can remove it if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe next time we do this we use "_highend" and "_lowend" or something similar so the builder names don't change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the builder name is our current mechanism for starting a new series in SkiaPerf, which we want when we change the kind of device a benchmark runs on. We could investigate a new mechanism for that, and then we could leave the builder names the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or they could be different, but have some kind of stable way to identify them as Android from the name agnostic of the device hardware, so this doesn't need to be updated every time.
| import 'package:firebase_crashlytics/firebase_crashlytics.dart'; | ||
| import 'package:flutter/foundation.dart'; | ||
| import 'package:flutter/material.dart'; | ||
| import 'package:flutter/rendering.dart'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this change but fix the warning:
info: The import of 'package:flutter/rendering.dart' is unnecessary because all of the used elements are also provided by the import of 'package:flutter/material.dart'. (unnecessary_import at [flutter_dashboard] lib/main.dart:11)
Update the dashboard so "mokey" and "pixel_7pro" tasks are filtered/iconed as Android. flutter/flutter#148085
For example,
Linux_mokey old_gallery__transition_perfshould show up in the dashboard as an Android test, but its icon and filtering counts it as a Linux test:Also fix two unrelated warnings.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.