Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

Add Firebase Version to header#414

Merged
schmidt-sebastian merged 9 commits intomasterfrom
mrschmidt-firebaseheader
Nov 2, 2018
Merged

Add Firebase Version to header#414
schmidt-sebastian merged 9 commits intomasterfrom
mrschmidt-firebaseheader

Conversation

@schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Oct 23, 2018 •
edited
Loading

This PR allows the Admin SDK to pass its version to our SDK. We then add it as a header value to the existing x-goog-api-client header.

This looks like this:

x-goog-api-client: gl-node/v8.11.3 grpc/1.15.1 gax/0.20.0 gapic/0.18.0 gccl/0.18.0 fire/6.1.0

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 23, 2018
Feiyang1
Feiyang1 reviewed Oct 24, 2018
View reviewed changes
dev/src/index.ts
};

if (settings && settings.firebaseVersion) {
libraryHeader.libVersion += ' fire/' + settings.firebaseVersion;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Feiyang1
Feiyang1 reviewed Oct 24, 2018
View reviewed changes
dev/src/index.ts
@@ -279,11 +279,6 @@ export class Firestore {
* use `Timestamp` now and opt-in to this new behavior as soon as you can.
*/
constructor(settings?: Settings) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@schmidt-sebastian
Copy link
Contributor Author

schmidt-sebastian commented Oct 25, 2018

Ping :)

@alexander-fenster
Copy link
Contributor

alexander-fenster commented Nov 2, 2018

It looks the code will add fire/x.x.x, not firebase/x.x.x as the description states. LGTM if that's expected. If you can create an issue for googleapis/gapic-generator to have better support for multiple library-version pairs, it would be great.

@alexander-fenster alexander-fenster self-requested a review November 2, 2018 19:53
@schmidt-sebastian
Copy link
Contributor Author

schmidt-sebastian commented Nov 2, 2018

It looks the code will add fire/x.x.x, not firebase/x.x.x as the description states. LGTM if that's expected. If you can create an issue for googleapis/gapic-generator to have better support for multiple library-version pairs, it would be great.

We updated the code to use fire/. I changed the description to match this.

Issue: https://github.com/googleapis/gapic-generator/issues/2400

@schmidt-sebastian schmidt-sebastian merged commit 9a2f450 into master Nov 2, 2018
@jmdobry jmdobry deleted the mrschmidt-firebaseheader branch December 5, 2018 07:07
@schmidt-sebastian schmidt-sebastian mentioned this pull request Nov 4, 2025
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@Feiyang1 Feiyang1 Feiyang1 left review comments

@alexander-fenster alexander-fenster alexander-fenster approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@JustinBeckwith JustinBeckwith

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@schmidt-sebastian @alexander-fenster @Feiyang1 @JustinBeckwith @googlebot