Skip to content

Swift naming StorageVersionString #560

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 1 commit into from
Apr 11, 2018
Merged

Swift naming StorageVersionString #560

merged 1 commit into from
Apr 11, 2018

Conversation

ulukaya
Copy link
Member

@ulukaya ulukaya commented Dec 12, 2017

No description provided.

Copy link
Member

@ryanwilson ryanwilson left a comment

Choose a reason for hiding this comment

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

This LGTM but since it's a breaking change, we should wait until next major version change. On that note - should we have a separate "breaking-changes" branch (Firestore has their own) to merge into. Thoughts?

@wilhuff
Copy link
Contributor

wilhuff commented Dec 14, 2017

If you do want a breaking-changes branch it can't be the firestore-api-changes branch since that branch will be short-lived. That branch is batching up a series of changes for the next launch vehicle available once we're ready. Firestore is still beta with a 0.x version so we can do this. The firestore-api-changes branch exists at all only because we want to minimize the number of breaking releases users might encounter.

@ryanwilson
Copy link
Member

Right right, thanks! In that case, we should have one but not include Firestore.

@ulukaya
Copy link
Member Author

ulukaya commented Dec 14, 2017

@wilhuff this is FirebaseStorage, not CloudStore.
@ryanwilson yeah, branch may work, or this PR can stay till next version. Either way, don't think devs were supposed to use this variable. I may be wrong.
Only realized that our ref doc was showing this with new name (because of post processing script), vs the new ref doc generation showed still as FIR. I thought it was a mistake on the script till I saw this in repo. I'll fix the ref doc manually for now.

@wilhuff
Copy link
Contributor

wilhuff commented Dec 14, 2017

@ulukaya Agreed that this is about storage. I was merely pointing out that it should stay separate from Firestore -- that you shouldn't appropriate firestore-api-changes as a staging ground for your own breaking changes.

@paulb777
Copy link
Member

We need to make a coherent version naming strategy for all components. CocoaPods automatically generates version variables like this when it creates (dynamic) frameworks.

@paulb777
Copy link
Member

I was thinking about working on a consistent global Firebase versioning scheme for M25, but that's not going to happen.

With approval from @schmidt-sebastian, we should go ahead and merge this for M25

@paulb777 paulb777 added this to the M25 milestone Apr 11, 2018
@paulb777 paulb777 merged commit edc7faa into master Apr 11, 2018
@paulb777 paulb777 deleted the ulukaya-patch-1 branch April 11, 2018 20:07
minafarid pushed a commit to minafarid/firebase-ios-sdk that referenced this pull request Jun 6, 2018
@firebase firebase locked and limited conversation to collaborators Nov 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants