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.

Sign up for GitHub

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

Jump to bottom

Add SwiftUI performance quickstart #1238

Merged
merged 41 commits into from
Aug 4, 2021
Merged

Add SwiftUI performance quickstart #1238

merged 41 commits into from
Aug 4, 2021

Conversation

jrcrespoh
Copy link
Contributor

@jrcrespoh jrcrespoh commented Jul 30, 2021 •
edited
Loading

TODO:

  • Fix CI
  • Remove macOS platform
  • Update platform versions
  • Add upload functionality
  • Add UI tests
  • Add comments
  • Consider adding Performance custom metrics
  • Consider restructuring data flow
  • Consider vision

@google-cla google-cla bot added the cla: yes label Jul 30, 2021
@jrcrespoh jrcrespoh marked this pull request as ready for review July 31, 2021 00:12
morganchen12
morganchen12 reviewed Aug 2, 2021
View reviewed changes
Copy link
Contributor

@morganchen12 morganchen12 left a comment

Choose a reason for hiding this comment

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

Nice work! I left some feedback, but please wait for Maksym/Paul's review before merging.

...SwiftUIPerformanceQuickstart/PerformanceExample/PerformanceExample.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
.github/workflows/performance.yml Show resolved Hide resolved
performance/SwiftUIPerformanceQuickstart/PerformanceExample/Shared/MainView.swift Outdated Show resolved Hide resolved
performance/SwiftUIPerformanceQuickstart/PerformanceExample/Shared/MainView.swift Show resolved Hide resolved
performance/SwiftUIPerformanceQuickstart/PerformanceExample/Shared/Process.swift Show resolved Hide resolved
performance/SwiftUIPerformanceQuickstart/PerformanceExample/Shared/Process.swift Outdated Show resolved Hide resolved
performance/SwiftUIPerformanceQuickstart/PerformanceExample/Shared/Process.swift Show resolved Hide resolved
performance/SwiftUIPerformanceQuickstart/PerformanceExample/Shared/Process.swift Outdated Show resolved Hide resolved
paulb777
paulb777 approved these changes Aug 3, 2021
View reviewed changes
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

I'm ok to defer the comments except for the pod deintegrate to a future PR.

...SwiftUIPerformanceQuickstart/PerformanceExample/PerformanceExample.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
performance/SwiftUIPerformanceQuickstart/PerformanceExample/Podfile Outdated Show resolved Hide resolved
performance/SwiftUIPerformanceQuickstart/PerformanceExample/Podfile Outdated Show resolved Hide resolved
performance/SwiftUIPerformanceQuickstart/PerformanceExample/Podfile Outdated Show resolved Hide resolved
paulb777
paulb777 approved these changes Aug 4, 2021
View reviewed changes
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

The lingering Pods references are the result of running pod deintegrate with the project open in Xcode.

It can be fixed by editing manually or by opening the xcodeproj and deleting the Pods in the Project Navigator. It should be red.

...SwiftUIPerformanceQuickstart/PerformanceExample/PerformanceExample.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
...SwiftUIPerformanceQuickstart/PerformanceExample/PerformanceExample.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
@jrcrespoh jrcrespoh requested a review from paulb777 August 4, 2021 00:49
paulb777
paulb777 approved these changes Aug 4, 2021
View reviewed changes
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

💯

@jrcrespoh jrcrespoh merged commit 541f652 into master Aug 4, 2021
@jrcrespoh jrcrespoh deleted the jc-performance-swiftui branch August 4, 2021 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulb777 paulb777 paulb777 approved these changes

@ncooke3 ncooke3 Awaiting requested review from ncooke3

@granluo granluo Awaiting requested review from granluo

@morganchen12 morganchen12 Awaiting requested review from morganchen12

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jrcrespoh @paulb777 @morganchen12 @granluo