Add Firestore vector value binding#122
Conversation
|
Hi there! I noticed your recent PRs mention running E2E tests with Also, if that's the new mandatory process, should we add a comment to the other #109 , to ensure it follows the new expectations? |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Hey @Kapusch. Good question, and happy to add some context. The bindings in this repo have always been a bit of a moving target. They’re incomplete, and in some cases incorrect, which can lead to runtime failures. That’s been true even back in the Xamarin/Microsoft days. Updates were mostly reactive, and there’s been ongoing drift as the native Firebase APIs evolve. Since taking over, I’ve mostly continued that approach (fix what’s needed, add APIs on demand), mainly because there hasn’t been any real automated validation in place. A full audit just wasn’t practical before. Recently though, I started experimenting with a more systematic approach using Codex + updated tooling. I had it generate a drift audit (~400 potential issues), filter to likely runtime problems, and then build an E2E harness to actually prove failures in the simulator. From there it’s been a loop of fix → validate → repeat. I'm in the early stages of letting it prove itself / trusting it. ⸻ Re: your question The run-firebase-foundation.sh E2E setup is part of that experiment. It’s not a formal/required contributor workflow yet. t’s still evolving pretty quickly. (I'm in the red -> green phase of red -> green -> refactor) I do think we’ll want to document it in CONTRIBUTING once things settle, but I’d rather not lock it in prematurely. ⸻ Re: #109 I don’t think we need to enforce this retroactively yet, but I’ll likely start encouraging it for new PRs once the workflow stabilizes. Also #109 is unfortunately not entirely correct. I'd like to get a few of these low-hanging, high-impact fixes in and then the plan is to release 12.6 -> 12.12 (or current) incrementally very soon. ⸻ Big picture, this is heading toward a more repeatable “detect → prove → fix” loop, which should make the project a lot more reliable over time, and less costly to maintain. |
Summary
FIRVectorValuebinding.FieldValue.VectorWithArray(NSNumber[])for the nativevectorWithArray:selector.Validation
dotnet pack source/Firebase/CloudFirestore/CloudFirestore.csproj --configuration Release --output outputtools/e2e/run-firebase-foundation.sh --package-dir output --configuration Debug --runtime-drift-case cloudfirestore-fieldvalue-vectorwitharraytools/e2e/run-firebase-foundation.sh --package-dir output --configuration Debuggit diff --checkNotes
+ (FIRVectorValue *)vectorWithArray:(NSArray<NSNumber *> *)array,FIRVectorValue.array, andinitWithArray:.