Partial wrapping of pb_ostream_t (pt1) #899

rsgowman · 2018-03-09T15:28:45Z

Wraps the varint based encode methods.

rsgowman · 2018-03-09T15:30:08Z

I'm currently about 4 commits into this. See the branch rsgowman/refactor_ostream if you want more context on where this is going. (I can move all those commits into this PR if it would be useful, but will otherwise keep them separate.)

zxu123

LGTM

var-const

LGTM with minor nits.

var-const · 2018-03-09T17:20:13Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

-  bool status = pb_encode_varint(stream, value);
+class PbOstream {
+ public:
+  explicit PbOstream(pb_ostream_t* stream) : stream_(stream) {


Do you intend to make this class create the stream on its own later on? I understand why it can't be done in this PR, but it seems to me to be worthwhile.

Yes. (In fact, it's already done. It's about 4 or so commits down the path.)

var-const · 2018-03-09T17:22:32Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

 */
-void EncodeVarint(pb_ostream_t* stream, uint64_t value) {
-  bool status = pb_encode_varint(stream, value);
+class PbOstream {


Nit: this name just mirrors the Nanopb C-ish convention. I don't have a good suggestion ready, though, so feel free to leave as-is. In particular, I think Pb part is unnecessary, and it's a little too abbreviated. OutputStream, maybe?

I don't have strong feelings on this either. Obviously, I just munged pb_ostream_t to PbOstream and called it a day.

I like having something to indicate that this is an output stream for a protocol buffer... and "PB" seemed as good an acronym as any. I suppose the full name might be ProtocolBufferOutputStream but that seems a little wordy.

In my view, the protobuf part is kinda implied because it's in serializer, but I don't feel strongly.

WriteStream? Writer?

This type is entirely local to the serializer so I don't think there's a whole lot of benefit to being super verbose here.

FWIW, I like Writer.

Suggestions are:

PbOstream, OutputStream, WriteStream, Writer

I'll keep the 'Stream' part, since nobody seems to hate that.

Output vs Writer: I don't care. I don't obviously prefer Writer, so I guess I'll stick with Output. (If you prefer Writer to Output, i.e. you weren't just floating another option, then I'll use Writer.)

Final name (for now): OutputStream.

Update: Costa likes Writer. That's two votes for Writer, so I'll use that.

var-const · 2018-03-09T17:25:39Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

+}
+
+void PbOstream::EncodeSize(size_t size) {
+  return EncodeVarint(size);


I presume you return void intentionally? (I'm fine with it, just checking)

Yes. Eventually, void->error_code throughout. (probably.)

var-const · 2018-03-09T17:27:18Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

-        abort();
-      }
-      EncodeInteger(stream, field_value.integer_value());
+      stream.EncodeTag(PB_WT_VARINT,


Can tag encoding be moved into the Encode* methods? Or is it realistically possible for a value encoded with EncodeVarint to be tagged something other than PB_WT_VARINT?

It could. (In fact, it used to be.) But we pulled it back out due to an asymmetry with the decode methods.

tl;dr for the decode is that while EncodeVarint/Bool/Int all use PB_WT_VARINT (so mapping is easy) but the reversing it is tricky since it's difficult to know if a PB_WT_VARINT should be decoded as a varint/bool/int. (EncodeString/Submessage also have the same problem as they both map to PB_WT_STRING.)

This is something we can take another look at, but it's probably easiest to do that after the rest of the refactoring (incl decoding) is done.

Agreed, let's revisit further on.

var-const · 2018-03-09T17:38:47Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

+
+  void EncodeSize(size_t size);
+  void EncodeNull();
+  void EncodeBool(bool bool_value);


I presume you don't use overloads to avoid unexpected conversions, right? I still think that something like

void Encode(bool); void Encode(int64_t); template <typename T> void Encode(T) = delete;

might be interesting, because it could open the door to making code more generic. If there's no opportunity for that, though, then it's not worth it.

Since this is kinda an 'ostream' like thing, you could even replace Encode* with operator<<. :)

For now, I've left this as being explicit. (I believe we're trying to err on the side of too explicit for internal apis.)

Please don't make this a std::ostream-like thing. We don't want any ADL in here nor do we want anything defined specifically for ostream to get involved.

We can push to make this more generic when we need it, but likely YAGNI.

wilhuff · 2018-03-09T21:44:46Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

 */
-void EncodeVarint(pb_ostream_t* stream, uint64_t value) {
-  bool status = pb_encode_varint(stream, value);
+class PbOstream {


WriteStream? Writer?

This type is entirely local to the serializer so I don't think there's a whole lot of benefit to being super verbose here.

wilhuff · 2018-03-09T21:47:31Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

+
+  void EncodeSize(size_t size);
+  void EncodeNull();
+  void EncodeBool(bool bool_value);


Please don't make this a std::ostream-like thing. We don't want any ADL in here nor do we want anything defined specifically for ostream to get involved.

We can push to make this more generic when we need it, but likely YAGNI.

wilhuff · 2018-03-09T21:50:53Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

+}
+
+void PbOstream::EncodeVarint(uint64_t value) {
+  bool status = pb_encode_varint(stream_, value);


Just to be sure: error handling changes are coming subsequent to this PR, yes?

wilhuff · 2018-03-09T21:54:27Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

@@ -291,7 +336,7 @@ void EncodeNestedFieldValue(pb_ostream_t* stream,
  size_t size = substream.bytes_written;

  // Write out the size to the output stream.
-  EncodeVarint(stream, size);
+  PbOstream(stream).EncodeSize(size);


This may just be a hot take on my part, but it seems that your abstraction over pb_ostream_t is incomplete. What if instead of taking a paramter of pb_ostream_t* you took a PbOstream* (or whatever you call it)?

your abstraction over pb_ostream_t is incomplete.

Deliberately so, yes. There's about 6 more commits in this series (and counting). Each quite small.

What if instead of taking a paramter of pb_ostream_t* you took a PbOstream* (or whatever you call it)?

Already done (in a future commit.)

wilhuff · 2018-03-09T22:37:16Z

Firestore/core/src/firebase/firestore/remote/serializer.cc

+   * <parentNameSpace>_<childNameSpace>_<message>_<field>_tag, e.g.
+   * google_firestore_v1beta1_Document_name_tag.
+   */
+  void EncodeTag(pb_wire_type_t wiretype, uint32_t field_number);


I know this is super annoying but I would expect that writers write, not encode.

Feel free to push this off with a todo or whatever until we've progressed down more of your review stack.

Added TODO.

Import of firebase-ios-sdk from Github. - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com> - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman@google.com> - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman@google.com> - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com> - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman@google.com> - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman@google.com> - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu@google.com> - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman@google.com> - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg@google.com> - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien@google.com> - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien@google.com> - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu@google.com> - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com> - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu@google.com> - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman@google.com> - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang@google.com> - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman@google.com> - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu@google.com> - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman@google.com> ORIGINAL_AUTHOR=Gil <wilhuff@github.com> PiperOrigin-RevId: 188809445

Import of firebase-ios-sdk from Github. - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com> - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman@google.com> - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman@google.com> - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com> - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman@google.com> - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman@google.com> - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu@google.com> - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman@google.com> - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg@google.com> - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien@google.com> - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien@google.com> - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu@google.com> - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com> - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu@google.com> - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman@google.com> - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang@google.com> - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman@google.com> - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu@google.com> - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman@google.com> PiperOrigin-RevId: 188809445

Import of firebase-ios-sdk from Github. - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com> - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman@google.com> - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman@google.com> - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com> - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman@google.com> - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman@google.com> - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu@google.com> - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman@google.com> - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg@google.com> - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien@google.com> - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien@google.com> - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu@google.com> - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com> - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu@google.com> - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman@google.com> - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang@google.com> - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman@google.com> - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu@google.com> - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman@google.com> ORIGINAL_AUTHOR=Gil <wilhuff@github.com> PiperOrigin-RevId: 188809445

Import of firebase-ios-sdk from Github. - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com> - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman> - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman> - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com> - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman> - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman> - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu> - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman> - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg> - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien> - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien> - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu> - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com> - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu> - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman> - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang> - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman> - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu> - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman> ORIGINAL_AUTHOR=Gil <wilhuff@github.com> PiperOrigin-RevId: 188809445

-- 197713382 by zxu <zxu>: Implement more on listener class and implement ListenerRegistration. TESTED: blaze test //sensored/testapp:testapp_build_test blaze test //sensored/testapp:testapp_build_test --config=android_x86 blaze test //sensored/testapp:android_testapp_test --config=android_x86 --define firebase_build=stable blaze test //sensored/cpp:firestore_kokoro blaze test //sensored/cpp:listener_registration_test -- 196551381 by zxu <zxu>: Implement more on listener class and implement the DocumentReference::AddSnapshotListener. Also added a few test case and fixed a bug in DocumentReferenceInternal. ListenerRegistration will be implemented in subsequent CL. TESTED: blaze test //sensored/testapp:testapp_build_test blaze test //sensored/testapp:testapp_build_test --config=android_x86 blaze test //sensored/testapp:android_testapp_test --config=android_x86 --define firebase_build=stable blaze test //sensored/cpp:firestore_kokoro blaze test //sensored/cpp:firestore_test -- 196276752 by zxu <zxu>: Implement the SnapshotMetadata with inline methods and (non-)Wrapper for Firestore C++. TESTED: blaze test //sensored/testapp:testapp_build_test blaze test //sensored/testapp:testapp_build_test --config=android_x86 blaze test //sensored/testapp:android_testapp_test --config=android_x86 --experimental_one_version_enforcement=warning blaze test //sensored/cpp:firestore_kokoro blaze test //sensored/cpp:firestore_test -- 195841793 by zxu <zxu>: Implement the wrapper class for callback (EventListener). People unlike huge CL. So sending this short one for review. In subsequent CLs, I'll do: * cache/uncache or the CppEventListener class and the wiring of native method; * actually using this to implement DocumentReference::AddSnapshotListener; * update the testapp to run android_test with callback. -- 194112388 by zxu <zxu>: Add Android-Wrapper for DocumentReference's non-callback methods. Tested: blaze test //sensored/testapp:testapp_build_test blaze test //sensored/testapp:testapp_build_test --config=android_x86 blaze test //sensored/cpp:firestore_kokoro blaze test //sensored/cpp:firestore_test -- 192445183 by zxu <zxu>: Add Android-Wrapper for Firestore's remaining methods. Tested: blaze test //sensored/testapp:testapp_build_test blaze test //sensored/testapp:testapp_build_test --config=android_x86 blaze test //sensored/cpp:firestore_kokoro blaze test //sensored/cpp:firestore_test -- 190986604 by mcg <mcg>: Manually import the public portion of - 22c226a C++ migration: make Timestamp class a part of public API ... by Konstantin Varlamov <var-const@users.noreply.github.com> Note that this only imports the header, which otherewise won't come over with a regular copybara run because we're currently excluding the public headers from the Firestore import with cl/188810622. The .cc implementation of this will come over with a regular copybara import, coming shortly. -- 189013767 by zxu <zxu>: Add Android-Wrapper for Firestore's method that does not involve other Firestore types. Tested: blaze test //sensored/testapp:testapp_build_test blaze test //sensored/testapp:testapp_build_test --config=android_x86 blaze test //sensored/cpp:firestore_kokoro -- 188809445 by mcg <mcg>: Import of firebase-ios-sdk from Github. - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com> - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman> - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman> - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com> - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman> - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman> - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu> - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman> - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg> - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien> - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien> - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu> - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com> - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu> - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman> - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang> - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman> - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu> - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman> -- 187049498 by mcg <mcg>: Import of firebase-ios-sdk from Github. - f7d9f29 Fix lint warnings (firebase#840) by Gil <mcg> - b5c60ad Refactor [En|De]codeVarint to be symetric wrt tags (firebase#837) by rsgowman <rgowman> - ddc12c0 Merge pull request firebase#694 from morganchen12/auth-docs by Morgan Chen <morganchen12@gmail.com> - 442d46f Avoid warnings about failing to override a designated ini... by Gil <mcg> - 4dc63f8 Fix Firestore tests for M22 (firebase#834) by Gil <mcg> - 935f3ca Avoid wrapping and rewrapping NSStrings when constructin... by Gil <mcg> - 6ce954a Serialize (and deserialize) int64 values (firebase#818) (firebase#829) by rsgowman <rgowman> - 41dcd4b Fix trivial mem leak in the test suite (firebase#828) by rsgowman <rgowman> - 50f9df9 Accept FIRTimestamp where NSDate is currently accepted as... by Konstantin Varlamov <var-const@users.noreply.github.com> - 67b068e Fix implicit retain self warnings (firebase#808) by rsgowman <rgowman> - 14ea068 Make FSTTimestamp into a public Firestore class (firebase#698) by Konstantin Varlamov <var-const@users.noreply.github.com> - de00de2 [En|De]codeUnsignedVarint -> [En|De]codeVarint (firebase#817) by rsgowman <rgowman> - 9bf73ab Fix two stream close issues (b/73167987, b/73382103). (firebase#8... by Michael Lehenbauer <mikelehen@gmail.com> - 7a4a2ea replacing FSTGetTokenResult by C++ Token implementation (... by zxu <zxu> - a9f3f35 Delete stale Firestore instances after FIRApp is deleted.... by Ryan Wilson <wilsonryan> - aa6f1ae Disable -Wrange-loop-analysis for abseil (firebase#807) by rsgowman <rgowman> - ef55eef Require official 1.4.0 for min CocoaPods version (firebase#806) by Paul Beusterien <paulbeusterien> - 7cddb97 Serialize (and deserialize) bool values (firebase#791) by rsgowman <rgowman> - 7a97f6c Enable -Wcomma for our build; disable it for abseil. (firebase#799) by rsgowman <rgowman> - 81ee594 DispatchQueue delayed callback improvements + testing (firebase#7... by Michael Lehenbauer <mikelehen@gmail.com> - fd9fd27 replacing Auth/FSTUser by C++ auth implementation (firebase#804) by zxu <zxu> - 6889850 Merge pull request firebase#801 from firebase/release-4.9.0 by Paul Beusterien <paulbeusterien> - e4be254 Fixed capitalization of init in test function. (firebase#797) by Ryan Wilson <wilsonryan> - 933be73 Improve documentation on auto-init property in FCM. (firebase#792) by Chen Liang <chliang> - 0f3c24b Actually ignore events on inactive streams, rather than j... by Greg Soltis <gsoltis> - fe19fca Serialize and deserialize null (firebase#783) by rsgowman <rgowman> - 95d0411 fix flaky test (firebase#788) by zxu <zxu> - b6613bd Cleaning up implicit retain for the RTDB and Storage by Sebastian Schmidt <mrschmidt> - 5f5f808 Keep track of number of queries in the query cache (firebase#776) by Greg Soltis <gsoltis> - c7c51a7 Fix double parsing on 32 bit devices. (firebase#787) by Ryan Wilson <wilsonryan> - 9504582 Port Firestore Document to C++ (firebase#777) by zxu <zxu> - adf9fb3 Update FieldValue of type Reference (firebase#775) by zxu <zxu> - 4afcfb1 Move to explicit nil check. (firebase#782) by Ryan Wilson <wilsonryan> - fd83e07 Strawman C++ API (firebase#763) by rsgowman <rgowman> - 8003348 C++ port: add C++ equivalent of FSTDocumentKey. (firebase#762) by Konstantin Varlamov <var-const@users.noreply.github.com> - 612d99c Fix Core CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF warnings (#... by Paul Beusterien <paulbeusterien> - bf45a15 port Firestore SnapshotVersion in C++ (firebase#767) by zxu <zxu> - d70c23e port Firestore Auth module in C++ (firebase#733) by zxu <zxu> - 633eb7b Let Travis run for `CMake` test and `lint.sh` (firebase#769) by zxu <zxu> - 274fe52 cmake build fixes (firebase#770) by rsgowman <rgowman> -- 184568931 by mcg <mcg>: Import of firebase-ios-sdk from Github. - 32266c5 Make sure Firestore/core/include is in the podspec (firebase#748) by Gil <mcg> - 070c0ea Merge pull request firebase#746 from morganchen12/unguarded-block by Morgan Chen <morganchen12@gmail.com> - 13f572e Increase expectation timeout to 25 seconds. (firebase#744) by Michael Lehenbauer <mikelehen@gmail.com> - 47d81fd fix (firebase#739) by Chen Liang <chliang> - 515625c Remove predecessorKey,Object,Document, etc (firebase#735) by Gil <mcg> - ac30522 Start on ArraySortedMap in C++ (firebase#721) by Gil <mcg> - 729b8d1 Move all Firestore Objective-C to Objective-C++ (firebase#734) by Gil <mcg> - 693d064 Merge pull request firebase#732 from OleksiyA/FCM-subscribe-compl... by Oleksiy Ivanov <oleksiy@informationrd.com> - 3cbdbf2 Schema migrations for LevelDB (firebase#728) by Greg Soltis <gsoltis> - 6474a82 Firestore DatabaseId in C++ (firebase#727) by zxu <zxu> - 05ef5ae Add absl_strings to firebase_firestore_util_test dependen... by rsgowman <rgowman> - 03a0069 Add changelog entry for my last PR (oops) and also add a ... by Michael Lehenbauer <mikelehen@gmail.com> - 5a280ca Import iterator_adaptors from google3 (firebase#718) by Gil <mcg> - 63a380b Use fixed-sized types (firebase#719) by Gil <mcg> - 6dc1373 Version updates to 4.8.2 (firebase#722) by Paul Beusterien <paulbeusterien> - a74d39f Fix a number of c++ build errors (firebase#715) by rsgowman <rgowman> - cceeec3 travis: check for copyright in sources (firebase#717) by Paul Beusterien <paulbeusterien> - 4d7c973 Fix b/72502745: OnlineState changes cause limbo document ... by Michael Lehenbauer <mikelehen@gmail.com> - af6976a normalize and port the rest of Firebase/Port code (firebase#713) by zxu <zxu> - 7226b4a port TargetIdGenerator to iOS (firebase#709) by zxu <zxu> - 5306474 adding unit test for auto init enable function (firebase#710) by Chen Liang <chliang> - 821fb90 implement `TargetIdGenerator` in C++ for Firestore (firebase#701) by zxu <zxu> - 5fdda3f normalize string_util (firebase#708) by zxu <zxu> - 15a2926 Update CHANGELOG for M21.1 release (firebase#679) by Ryan Wilson <wilsonryan> - f027214 Fix incorrect deprecation message (firebase#688) by Iulian Onofrei <6d0847b9@opayq.com> - bfa0e40 Implement the rest of FieldValue types for C++ (firebase#687) by zxu <zxu> - 1f77212 Properly publish Abseil sources as a part of the podspec ... by Gil <mcg> ORIGINAL_AUTHOR=Firebase <firebase-noreply> PiperOrigin-RevId: 197713382

Wraps the varint based encode methods.

Partial wrapping of pb_ostream_t

2d45f76

Wraps the varint based encode methods.

googlebot added the cla: yes label Mar 9, 2018

rsgowman requested review from var-const and zxu123 March 9, 2018 15:30

zxu123 reviewed Mar 9, 2018

View reviewed changes

var-const approved these changes Mar 9, 2018

View reviewed changes

rsgowman requested a review from wilhuff March 9, 2018 20:42

rsgowman assigned wilhuff Mar 9, 2018

wilhuff reviewed Mar 9, 2018

View reviewed changes

PbOstream -> Writer

8225f67

wilhuff approved these changes Mar 9, 2018

View reviewed changes

Add TODO to rename Encode* -> Write*

ceef0ae

wilhuff assigned rsgowman and unassigned wilhuff Mar 9, 2018

rsgowman merged commit 0d3adb1 into master Mar 9, 2018

rsgowman deleted the rsgowman/refactor_ostream_pt1 branch March 9, 2018 23:16

zxu123 mentioned this pull request May 23, 2018

Firestore project: Import of API interfaces. zxu123/firebase-ios-sdk#3

Closed

minafarid pushed a commit to minafarid/firebase-ios-sdk that referenced this pull request Jun 6, 2018

Partial wrapping of pb_ostream_t (pt1) (firebase#899)

f9256c6

Wraps the varint based encode methods.

firebase locked and limited conversation to collaborators Nov 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partial wrapping of pb_ostream_t (pt1) #899

Partial wrapping of pb_ostream_t (pt1) #899

rsgowman commented Mar 9, 2018

rsgowman commented Mar 9, 2018

zxu123 left a comment

var-const left a comment

var-const Mar 9, 2018

rsgowman Mar 9, 2018

var-const Mar 9, 2018

rsgowman Mar 9, 2018

var-const Mar 9, 2018

wilhuff Mar 9, 2018

var-const Mar 9, 2018

rsgowman Mar 9, 2018

rsgowman Mar 9, 2018

var-const Mar 9, 2018

rsgowman Mar 9, 2018

var-const Mar 9, 2018

rsgowman Mar 9, 2018

var-const Mar 9, 2018

var-const Mar 9, 2018

rsgowman Mar 9, 2018

wilhuff Mar 9, 2018

wilhuff Mar 9, 2018

wilhuff Mar 9, 2018

wilhuff Mar 9, 2018

rsgowman Mar 9, 2018

wilhuff Mar 9, 2018

rsgowman Mar 9, 2018

wilhuff Mar 9, 2018

rsgowman Mar 9, 2018

Partial wrapping of pb_ostream_t (pt1) #899

Partial wrapping of pb_ostream_t (pt1) #899

Conversation

rsgowman commented Mar 9, 2018

rsgowman commented Mar 9, 2018

zxu123 left a comment

Choose a reason for hiding this comment

var-const left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment