Skip to content

Add expected count to Target #10872

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

Conversation

milaGGL
Copy link
Contributor

@milaGGL milaGGL commented Feb 27, 2023

#no-changelog

  • make expected_count optional in proto
  • add expected_count to Target only when resume token is presented(and set has_expected_count to true)
  • port expected_count related spec tests from web and make sure it is not gonna break

@milaGGL milaGGL changed the base branch from master to mila/BloomFilter February 27, 2023 23:33
@milaGGL milaGGL self-assigned this Feb 27, 2023
@google-oss-bot
Copy link

Coverage Report 1

Affected Products

  • FirebaseFirestore-iOS-FirebaseFirestore.framework

    Overall coverage changed from 88.26% (61db372) to 88.22% (fe7b3e0) by -0.04%.

    FilenameBase (61db372)Merge (fe7b3e0)Diff
    bloom_filter.nanopb.cc?0.00%?
    firebase_metadata_provider_apple.mm86.96%100.00%+13.04%
    firestore.nanopb.cc35.71%35.73%+0.01%
    remote_store.cc90.86%91.06%+0.20%
    serializer.cc91.05%91.09%+0.04%
    target_data.cc40.63%48.00%+7.37%
    task.cc94.78%93.91%-0.87%
    wrappers.nanopb.cc9.40%11.11%+1.71%
    write.nanopb.cc60.23%61.27%+1.04%
    write_stream.cc91.55%94.37%+2.82%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/Lu5Na6uOSR.html

@milaGGL milaGGL requested a review from dconeybe February 28, 2023 21:30
@milaGGL milaGGL marked this pull request as ready for review February 28, 2023 21:30
@milaGGL milaGGL requested a review from dconeybe March 2, 2023 02:53
@dconeybe
Copy link
Contributor

dconeybe commented Mar 3, 2023

Note: Merge in the base branch after you merge #10874 (Add golden tests for BloomFilter). Doing so will fix the "firestore / cmake (ubuntu-latest) (pull_request)" GitHub action, which is failing because CommonCrypto is not available on Linux (https://github.com/firebase/firebase-ios-sdk/actions/runs/4318256537/jobs/7536444988)

@milaGGL milaGGL requested a review from dconeybe March 6, 2023 21:16
@milaGGL milaGGL requested a review from dconeybe March 7, 2023 22:43
@milaGGL milaGGL merged commit 3b5b49b into mila/BloomFilter Mar 8, 2023
@milaGGL milaGGL deleted the mila/BloomFilter-add-expected-count-to-Target branch March 8, 2023 17:02
@firebase firebase locked and limited conversation to collaborators Apr 8, 2023
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.

3 participants