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

[persistence] Clarify intention of FilterCriteria beginDate/endDate #3164

Merged
merged 1 commit into from
Nov 21, 2022
Merged

[persistence] Clarify intention of FilterCriteria beginDate/endDate #3164

merged 1 commit into from
Nov 21, 2022

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Nov 17, 2022

See openhab/openhab-addons#9906 (comment)

Most persistence implementations filters by beginDate <=date <= endDate which contradicts the Javadoc comments with proposed changes in this PR. The exception is JDBC persistence which lead to openhab/openhab-addons#9906.

Therefore, I would propose to either:

@jlaur
Clarify intention of beginDate/endDate
752ad36 
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur requested a review from a team as a code owner November 17, 2022 21:54
cweitkamp
cweitkamp approved these changes Nov 18, 2022
View reviewed changes
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

I am fine with the change of this definition. Let's ask one more @openhab/core-maintainers about it.

J-N-K
J-N-K approved these changes Nov 18, 2022
View reviewed changes
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

I agree. Let's wait some days if someone else differs and merge on Tuesday if no one stands up against.

kaikreuzer
kaikreuzer approved these changes Nov 18, 2022
View reviewed changes
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

I'm also fine with it and would even assume that this was the original intention.

@J-N-K J-N-K merged commit bf38f4d into openhab:main Nov 21, 2022
@J-N-K J-N-K added this to the 3.4 milestone Nov 21, 2022
@jlaur jlaur deleted the persistence-filtercriteria-comment branch November 21, 2022 18:39
@J-N-K J-N-K mentioned this pull request May 7, 2023
Merged
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
@jlaur @splatch
Clarify intention of beginDate/endDate in persistence extensions (ope…
ee4c086 
…nhab#3164)

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
GitOrigin-RevId: bf38f4d
@jlaur jlaur mentioned this pull request Jul 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wborn wborn wborn approved these changes

@J-N-K J-N-K J-N-K approved these changes

@cweitkamp cweitkamp cweitkamp approved these changes

@kaikreuzer kaikreuzer kaikreuzer approved these changes

Assignees
No one assigned
Labels
documentation persistence
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
5 participants
@jlaur @kaikreuzer @cweitkamp @J-N-K @wborn