-
Notifications
You must be signed in to change notification settings - Fork 488
create sample chat app #89
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
ae9451c to
d29b101
Compare
| CLANG_ENABLE_MODULES = YES; | ||
| INFOPLIST_FILE = uidemo/Info.plist; | ||
| LD_RUNPATH_SEARCH_PATHS = "$(inherited) @executable_path/Frameworks"; | ||
| PRODUCT_BUNDLE_IDENTIFIER = com.firebase.uidemo; |
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.
make the bundle something like "com.google.firebase.firebaseui.uidemo" to be more descriptive, uideo is no t a Firebase product but a demo under FirebaseUI project
| /* End PBXContainerItemProxy section */ | ||
|
|
||
| /* Begin PBXFileReference section */ | ||
| 1A128E4DD412913B07E20C37 /* Pods_uidemoTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_uidemoTests.framework; sourceTree = BUILT_PRODUCTS_DIR; }; |
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.
Deintegrate before checking in, else we will have issues when pod versions 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.
what's deintegrate?
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.
pod deintegrate - it is a plugin (bundled from 1.0) that removes all of the cocoapods specific stuff from the pbxproj. Shouldn't chance the basic flow (pod update/install then go), but means you don't hit silliness when pod install is run again a project that already has pod stuff from an older version (e.g. if the names of build phases change due to emoji boxes being added or similar, you end up with two sets of build phases and sadness ensues)
samples/swift/uidemo/Sample.swift
Outdated
|
|
||
| import UIKit | ||
|
|
||
| // As we add more sample use cases to FirebaseUI, |
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.
This describes the intent, but in a roundabout way. Describe what it is, and then how it should be used.
|
Code should be ready for another round of review. |
| isa = PBXNativeTarget; | ||
| buildConfigurationList = 8DABC9A21D3D82D600453807 /* Build configuration list for PBXNativeTarget "uidemo" */; | ||
| buildPhases = ( | ||
| 757DE2C28DE66EF8748CA3DA /* [CP] Check Pods Manifest.lock */, |
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.
DId these get left behind even after a pod deintegrate? Interesting if so.
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.
Yeah, seems like it.
|
Couple of nits, but otherwise LGTM. |
|
Should be good to merge now. |
Hoping achieve sample parity with FirebaseUI-Android soon.
This code is going to serve as a reference for every developer who tries to use this library, so I'm not quite happy with where it is yet (it could use more comments, for starters)--but I think it's far along enough for some code review.