Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Markduckworth/or queries pr 4 #6527

Merged
merged 35 commits into from
Oct 26, 2022
Merged

Markduckworth/or queries pr 4 #6527

merged 35 commits into from
Oct 26, 2022

Conversation

MarkDuckworth
Copy link
Contributor

Hey there! So you want to contribute to a Firebase SDK?
Before you file this pull request, please read these guidelines:

Discussion

  • Read the contribution guidelines (CONTRIBUTING.md).
  • If this has been discussed in an issue, make sure to link to the issue here.
    If not, go file an issue about this before creating a pull request to discuss.

Testing

  • Make sure all existing tests in the repository pass after your change.
  • If you fixed a bug or added a feature, add a new test to cover your code.

API Changes

  • At this time we cannot accept changes that affect the public API. If you'd like to help
    us make Firebase APIs better, please propose your change in an issue so that we
    can discuss it together.

@changeset-bot
Copy link

changeset-bot bot commented Aug 12, 2022 •
edited
Loading

⚠️ No Changeset found

Latest commit: 2ddc068

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 12, 2022 •
edited
Loading

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (bf21b93)Merge (501d7c0)Diff
    browser266 kB269 kB+2.75 kB (+1.0%)
    esm5330 kB333 kB+3.21 kB (+1.0%)
    main531 kB538 kB+6.69 kB (+1.3%)
    module266 kB269 kB+2.75 kB (+1.0%)
    react-native266 kB269 kB+2.75 kB (+1.0%)

  • bundle

    TypeBase (bf21b93)Merge (501d7c0)Diff
    firestore (Persistence)275 kB277 kB+1.96 kB (+0.7%)
    firestore (Query Cursors)211 kB211 kB+37 B (+0.0%)
    firestore (Query)212 kB213 kB+47 B (+0.0%)
    firestore (Read data once)201 kB201 kB+37 B (+0.0%)
    firestore (Realtime updates)203 kB203 kB+37 B (+0.0%)
    firestore (Transaction)184 kB184 kB+55 B (+0.0%)
    firestore (Write data)184 kB184 kB+55 B (+0.0%)

  • firebase

    TypeBase (bf21b93)Merge (501d7c0)Diff
    firebase-compat.js740 kB742 kB+1.89 kB (+0.3%)
    firebase-firestore-compat.js315 kB317 kB+1.89 kB (+0.6%)
    firebase-firestore.js313 kB316 kB+2.75 kB (+0.9%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 12, 2022 •
edited
Loading

Size Analysis Report 1

This report is too large (604,118 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.

Test Logs

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

@MarkDuckworth MarkDuckworth marked this pull request as ready for review August 19, 2022 22:52
ehsannas
ehsannas reviewed Aug 29, 2022
View reviewed changes
Copy link
Contributor

@ehsannas ehsannas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. Just a few nits outlined below.

packages/firestore/src/core/query.ts Show resolved Hide resolved
packages/firestore/src/local/indexeddb_index_manager.ts Outdated Show resolved Hide resolved
packages/firestore/src/util/logic_utils.ts Outdated Show resolved Hide resolved
packages/firestore/src/util/logic_utils.ts Outdated Show resolved Hide resolved
packages/firestore/test/unit/core/query.test.ts Outdated Show resolved Hide resolved
packages/firestore/test/unit/core/query.test.ts Outdated Show resolved Hide resolved
packages/firestore/test/unit/util/logic_utils.test.ts Outdated Show resolved Hide resolved
@ehsannas ehsannas assigned MarkDuckworth and unassigned ehsannas Aug 29, 2022
ehsannas
ehsannas requested changes Oct 5, 2022
View reviewed changes
Copy link
Contributor

@ehsannas ehsannas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last thing

packages/firestore/src/core/target.ts Show resolved Hide resolved
@ehsannas ehsannas removed their assignment Oct 5, 2022
ehsannas
ehsannas approved these changes Oct 25, 2022
View reviewed changes
Copy link
Contributor

@ehsannas ehsannas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MarkDuckworth MarkDuckworth merged commit c15333a into markduckworth/or-queries Oct 26, 2022
@MarkDuckworth MarkDuckworth deleted the markduckworth/or-queries-pr-4 branch October 26, 2022 15:22
@firebase firebase locked and limited conversation to collaborators Nov 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ehsannas ehsannas ehsannas approved these changes

@dwyfrequency dwyfrequency Awaiting requested review from dwyfrequency

@hsubox76 hsubox76 Awaiting requested review from hsubox76

Assignees

@MarkDuckworth MarkDuckworth

@ehsannas ehsannas

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MarkDuckworth @google-oss-bot @ehsannas