Skip to content

[Sessions] Initial commit of Firebase Sessions SDK #10285

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

Merged
merged 2 commits into from
Oct 3, 2022

Conversation

samedson
Copy link
Contributor

@samedson samedson commented Sep 28, 2022

Paired with: @jeremyjiang-dev

Notes

  • We haven't setup any SDKs with the Sessions SDK as a dependency, so this should not go out to users (unless they specify it directly)
  • We've only setup CocoaPods so far (for development), not SPM

Questions

  • We were using FunctionsComponent.swift as a reference. We see there is a protocol and initializer there. Our protocol isn't being called, and I'm not sure where their functions(... protocol method is being called
    • Is the protocol needed?
    • It looks like it's needed as something to pass into the Component constructor

#no-changelog
#sessions

@samedson samedson requested a review from paulb777 September 28, 2022 15:54
@ncooke3 ncooke3 self-requested a review September 28, 2022 17:20
@ncooke3 ncooke3 self-assigned this Sep 28, 2022
@firebase firebase deleted a comment from google-oss-bot Sep 28, 2022
Copy link
Member

@ncooke3 ncooke3 left a comment

Choose a reason for hiding this comment

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

Still working through the DD... but here's a first pass!

@samedson
Copy link
Contributor Author

samedson commented Oct 3, 2022

Thanks for the suggestions @ncooke3 ! Added my responses.

@ncooke3
Copy link
Member

ncooke3 commented Oct 3, 2022

CI should be fixed by a rebase on top of master.

@samedson samedson force-pushed the firebase-sessions-first branch from 9c8c297 to 30bbe0e Compare October 3, 2022 15:30
@ncooke3
Copy link
Member

ncooke3 commented Oct 3, 2022

@samedson, sorry– I thought the rebase would surely fix CI. It's not a blocker though since the failure is unrelated.

@samedson samedson force-pushed the firebase-sessions-first branch from 1ac7495 to f5b5504 Compare October 3, 2022 16:27
@samedson samedson force-pushed the firebase-sessions-first branch from f5b5504 to b9195ef Compare October 3, 2022 16:27
@samedson
Copy link
Contributor Author

samedson commented Oct 3, 2022

Sounds good - thanks for the advice! I made one change to include a reference to the Google App ID because we'll need that

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.

While starting with CocoaPods is ok, it might be better to do early development in Swift Package Manager, since it's the future.

@samedson samedson enabled auto-merge (squash) October 3, 2022 19:37
@samedson samedson merged commit e04cb62 into master Oct 3, 2022
@samedson samedson deleted the firebase-sessions-first branch October 3, 2022 19:41
@samedson samedson added the sessions Changes pertaining to the Firebase Sessions SDK label Oct 3, 2022
@samedson samedson changed the title Initial commit of Firebase Sessions SDK [Sessions] Initial commit of Firebase Sessions SDK Oct 3, 2022
@firebase firebase locked and limited conversation to collaborators Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: core cla: yes sessions Changes pertaining to the Firebase Sessions SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants