Skip to content

feat: add isType expression#2329

Open
dlarocque wants to merge 6 commits intomainfrom
dl/istype-expression
Open

feat: add isType expression#2329
dlarocque wants to merge 6 commits intomainfrom
dl/istype-expression

Conversation

@dlarocque
Copy link
Contributor

Adds the isType expression, and the Type enum, which contains all the types that the Firestore backend can generate.

Question: The DocumentChange (DocumentChange.java) class already defines a Type enum that represents all snapshot diff types. Is it okay to have these two enum types in the SDK? I believe most users would use this type as DocumentChange.Type rather than just Type, so there should be no name collisions.

Adds the `isType` expression, and the `Type` enum, which contains all
the types that the Firestore backend can generate.

Question: The `DocumentChange` class defines a `Type` enum that
represents all snapshot diff types. Is it okay to have these two enum
types in the SDK?
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: firestore Issues related to the googleapis/java-firestore API. labels Feb 27, 2026
@dlarocque dlarocque marked this pull request as ready for review March 11, 2026 15:05
@dlarocque dlarocque requested review from a team as code owners March 11, 2026 15:05
@dlarocque dlarocque requested review from milaGGL and yvonnep165 March 11, 2026 15:05
// [END is_type]
System.out.println(result.getResults());
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think tech writers write the samples so we don't need to edit this file.

Copy link
Contributor

@yvonnep165 yvonnep165 left a comment

Choose a reason for hiding this comment

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

For the Type enum question, I would say ok to have the two enum types in the SDK. But I'm not 100% confident to say so. Will rely on Mila's opinion. Everything else looks good to me!

@milaGGL
Copy link
Contributor

milaGGL commented Mar 11, 2026

For the Type enum question, I would say ok to have the two enum types in the SDK. But I'm not 100% confident to say so. Will rely on Mila's opinion. Everything else looks good to me!

Good question, my gut says no, cause it is not a good practice to have 2 APIs of the same name and expect users to use prefix to differentiate them. Please use a more descriptive naming for this, maybe like DataType.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: firestore Issues related to the googleapis/java-firestore API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants