Ppl API #14513
Ppl API #14513
Conversation
google-oss-bot
added
api: abtesting
api: analytics
api: appcheck
api: appdistribution
api: auth
api: core
api: crashlytics
api: database
api: dynamiclinks
api: firestore
api: functions
api: inappmessaging
Generated by 🚫 Danger |
cherylEnkidu
force-pushed
the
cheryllin/pplapi
branch
2 times, most recently
from
735b71a to
3b9ca57
Compare
cherylEnkidu
force-pushed
the
cheryllin/pplapi
branch
from
3b9ca57 to
daf1b68
Compare
cherylEnkidu
force-pushed
the
cheryllin/pplapi
branch
4 times, most recently
from
bb1bc1c to
f5f8445
Compare
cherylEnkidu
force-pushed
the
cheryllin/pplapi
branch
from
f5f8445 to
5b2bd88
Compare
| @@ -40,6 +40,20 @@ void Pipeline::execute(util::StatusOrCallback<PipelineSnapshot> callback) { | |||
| this->firestore_->RunPipeline(*this, std::move(callback)); | |||
| } | |||
|
|
|||
| google_firestore_v1_Value Pipeline::to_proto() const { | |||
There was a problem hiding this comment.
Where is this used?
Serializer::EncodePipeline does the same thing. If you want to move it here, we should update the logic there.
| : expr_(std::move(expr)), field_name_(absl::nullopt) { | ||
| } | ||
| ReplaceWith::ReplaceWith(std::string field_name) | ||
| : expr_(nullptr), field_name_(std::move(field_name)) { |
There was a problem hiding this comment.
How does this stage work? I am not sure i understand the implementation. Is it:
- if expr_ is set, and if it is a map, it will replace the input document?
- if field_name_ is set, it is evaluated against the document, and if it is a map it will replace the input document?
| : orders_(std::move(orders)) { | ||
| } | ||
| ~SortStage() override = default; | ||
|
|
||
| google_firestore_v1_Pipeline_Stage to_proto() const override; | ||
|
|
||
| private: | ||
| std::vector<Ordering> orders_; | ||
| std::vector<std::shared_ptr<Ordering>> orders_; |
There was a problem hiding this comment.
Why a pointer is needed here? I think we only needed if you want polymorphism?
| .collection(path: "/foo") | ||
| .where(eq(field("foo"), constant("bar"))) | ||
| .collection("/foo") | ||
| .where(Field("foo") == Constant("bar")) |
There was a problem hiding this comment.
I thought we would move away from operator overload?
| "awards": ["hugo": true, "nebula": true], | ||
| ], | ||
| ] | ||
|
|
||
| @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) | ||
| class PipelineIntegrationTests: FSTIntegrationTestCase { |
There was a problem hiding this comment.
Do these tests pass locally at least?
|
|
||
| import FirebaseFirestore | ||
|
|
||
| final class PipelineTests: FSTIntegrationTestCase { |
There was a problem hiding this comment.
I do not think we need to keep this file around. It serves a purpose as a way to check for compilation errors, but we can just move them to the test below and actually run them.
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| enum Helper { |
There was a problem hiding this comment.
Why an enum instead of a struct or class? Is this a swift convention?
| import Foundation | ||
|
|
||
| // TODO: the implementation of `Expr` is not complete | ||
| public protocol Expr: Sendable { |
There was a problem hiding this comment.
All of these should have public documentation, with code samples.
| } | ||
| } | ||
|
|
||
| /// Adds new fields to outputs from previous stages. |
There was a problem hiding this comment.
I believe other platforms include code samples of how to use this stage. Please add them here as well.
No description provided.