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.

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

Firebase 3 + Naming Collisions fix #21

Closed
wants to merge 1 commit into from
Closed

Firebase 3 + Naming Collisions fix #21

wants to merge 1 commit into from

Conversation

Herakleis
Copy link

No description provided.

@Herakleis
Copy link
Author

Sorry guys, first commit ever on open source I think I did something wrong :s
Or maybe looks like Travis doesn't know about Firebase new types?
Also how can I clean the garbage commited files that are not actual code modifications?

@SD10
Copy link
Owner

SD10 commented Jul 10, 2017

@Herakleis Thank you for your pull request but I'm having trouble understanding the reasoning behind it. Did Firebase update their API recently? I just looked at the Firebase repo and it looks like all the types are still named the same. i.e) FIRDatabaseReference, FIRDataSnapshot, etc.

Ok, I see that their docs have been updated for new types. Nora + Travis are using an older version of Firebase.

@Herakleis
Copy link
Author

@SD10 They did update it, had to correct a lot of references :(
screen shot 2017-07-10 at 8 21 01 pm

@SD10
Copy link
Owner

SD10 commented Jul 10, 2017

@Herakleis Yeah thanks for pointing this out.
What version of FirebaseDatabase and FirebaseStorage are you using?

Right now this pull request presents a few inconsistencies in the naming conventions for the API.
i.e.) Some types are prefixed with NR and some are not.

I hadn't really planned on changing the API so this could take a day to mull over, but it is a priority.
I'm also not sure if I want to use a NR prefix or Nora.
😭 Firebase stole my clean naming conventions!

@Herakleis
Copy link
Author

@SD10 Using the most up-to-date !
Yup they also collided with my (and probably all the other's) User model lol since their FIRUser is now... User

@SD10
Copy link
Owner

SD10 commented Jul 11, 2017

@Herakleis Sorry I still haven't gotten around to this.
As for the new naming conventions... I agree its absurd.
FIRUser -> User
FIRDatabase -> Database
These names are too prone to conflicts.

@SD10
Copy link
Owner

SD10 commented Jul 13, 2017

@Herakleis I haven't had time to fix this. Would you be willing to make a few more changes for me?
I'd like every Nora type to have the NR prefix.

For example:

  • NoraError -> NRError
  • DatabaseProvider -> NRDatabaseProvider
  • StorageProvider -> NRStorageProvider

The only exception to this would be the Result type. I would like it to remain as Result.

In order to get Travis to build you will also need to upgrade the Podfile with the latest versions of Firebase. Please explicitly set the version numbers.

No worries if you can't get around to this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants