Skip to content

add macos project to macrobenchmarks - #50999

Merged
dnfield merged 4 commits into
flutter:masterfrom
dnfield:add_macos
Feb 20, 2020
Merged

dnfield merged 4 commits into
flutter:masterfrom
dnfield:add_macos

Conversation

@dnfield

@dnfield dnfield commented Feb 18, 2020

Copy link
Copy Markdown
Contributor

Adds a macos project to macrobenchmarks for local testing. We do not yet have any benchmarks for this.

/cc @liyuqian @stuartmorgan

@fluttergithubbot fluttergithubbot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Feb 18, 2020
@dnfield dnfield mentioned this pull request Feb 18, 2020
@liyuqian

Copy link
Copy Markdown
Contributor

I wonder why the original PR (#50851) has +1,317 lines while this sub-PR has +17,830 lines. Is everything in ephemeral necessary?

@dnfield

dnfield commented Feb 19, 2020

Copy link
Copy Markdown
Contributor Author

Ahh good catch, I'll fix that. I added macos and forgot to update the .gitignore.

@dnfield

dnfield commented Feb 19, 2020

Copy link
Copy Markdown
Contributor Author

I think this is right now.

@liyuqian liyuqian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

RSLGTM. Defer the final LGTM to the desktop experts.

@stuartmorgan-g stuartmorgan-g left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with two small things

@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file is a mistake that I didn't notice was added to the template. Someone just caught this and sent a PR recently that fixed the template, but this file should be removed here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed

PRODUCT_NAME = macrobenchmarks

// The application's bundle identifier
PRODUCT_BUNDLE_IDENTIFIER = com.example.macrobenchmarks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we use junk bundle ID and copyright in the iOS app? If not, these should be fixed up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes - these values match the iOS versions.

@fluttergithubbot

Copy link
Copy Markdown
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite hostonly_devicelab_tests-3_last-macos has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dnfield
dnfield merged commit 2ce51aa into flutter:master Feb 20, 2020
@dnfield
dnfield deleted the add_macos branch February 20, 2020 09:50
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants