Skip to content

Add and use a TimeRange class to represent before/after timestamp.#4719

Merged
auto-submit[bot] merged 1 commit into
flutter:mainfrom
matanlurey:firestore-filter-TimeRange
May 22, 2025
Merged

Add and use a TimeRange class to represent before/after timestamp.#4719
auto-submit[bot] merged 1 commit into
flutter:mainfrom
matanlurey:firestore-filter-TimeRange

Conversation

@matanlurey
Copy link
Copy Markdown
Contributor

I started to work on flutter/flutter#169234 and realized the current way timestamp filtering is done with Firebase is inflexible (and personally, confusing). This adds a small-ish TimeRange class in cocoon_commons and uses it instead of int? timestamp to determine which commits to find.

(Even if we want a different approach than flutter/flutter#169234, this seems worth it?)

@matanlurey matanlurey requested a review from jtmcdole May 21, 2025 22:34
Copy link
Copy Markdown
Member

@jtmcdole jtmcdole left a comment

Choose a reason for hiding this comment

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

What is inflexible about timestamp?

More specifically; we can add multiple query parameters to a timestamp:

image

Copy link
Copy Markdown
Member

@jtmcdole jtmcdole left a comment

Choose a reason for hiding this comment

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

Looked at the code and you are filtering, so it looks like a nice adapter on top.

factory TimeRange.between(
DateTime start, //
DateTime end, {
bool exclusive,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this either:
(start, end) or [start, end]
and never (start, end] or [start, end)

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App. label May 22, 2025
@auto-submit auto-submit Bot merged commit ee45870 into flutter:main May 22, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants