Skip to content

Conversation

@schmidt-sebastian
Copy link
Contributor

Gil asked that this should not be part of #2383, but I think it still makes sense as an indepedent simplification

Gil asked that this should not be part of #2383, but I think it still makes sense as an indepedent simplification
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 9, 2021

Coverage Report

Affected SDKs

  • firebase-firestore

    SDK overall coverage changed from 48.64% (1c62992) to 48.64% (707e57bb) by -0.00%.

    Filename Base (1c62992) Head (707e57bb) Diff
    PatchMutation.java 92.31% 92.16% -0.15%
    RemoteSerializer.java 84.20% 84.60% +0.40%
    SetMutation.java 84.85% 83.87% -0.98%

Test Logs

Notes

HTML coverage reports can be produced locally with ./gradlew <product>:checkCoverage.
Report files are located at <product-build-dir>/reports/jacoco/.

Head commit (707e57bb) is created by Prow via merging commits: 1c62992 7bfed6c.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 9, 2021

Binary Size Report

Affected SDKs

  • firebase-firestore

    Type Base (1c62992) Head (707e57bb) Diff
    aar 1.10 MB 1.10 MB -74 B (-0.0%)
    apk (release) 3.19 MB 3.19 MB -8 B (-0.0%)

Test Logs

Notes

Head commit (707e57bb) is created by Prow via merging commits: 1c62992 7bfed6c.

@google-oss-bot
Copy link
Contributor

@schmidt-sebastian: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
smoke-tests 7bfed6c link /test smoke-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Copy link

@thebrianchen thebrianchen left a comment

Choose a reason for hiding this comment

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

lgtm

transformResult != null
? Collections.singletonList(TestUtil.wrap(transformResult))
: null);
: Collections.emptyList());

Choose a reason for hiding this comment

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

optional nit: you can just remove the Nullable annotation in the signature and change this method's overload to pass in an empty list instead of null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, this would have been nice but unfortunately wrap throws when it is passed an empty Object.

@schmidt-sebastian schmidt-sebastian merged commit 8655b90 into master Feb 9, 2021
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/simplifytransform branch February 9, 2021 21:41
schmidt-sebastian added a commit to firebase/firebase-js-sdk that referenced this pull request Feb 25, 2021
@firebase firebase locked and limited conversation to collaborators Mar 12, 2021
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.

4 participants