-
Notifications
You must be signed in to change notification settings - Fork 21
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.
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
Remove custom tsrange
parser
#268
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tagliala
force-pushed
the
feature/267-remove-tsrange-override
branch
from
February 22, 2024 16:03
489a4fb
to
545fbbf
Compare
tagliala
force-pushed
the
feature/267-remove-tsrange-override
branch
from
February 22, 2024 16:19
545fbbf
to
47e8585
Compare
tagliala
force-pushed
the
feature/267-remove-tsrange-override
branch
from
February 23, 2024 11:48
47e8585
to
b6db434
Compare
gridanjbf
reviewed
Feb 23, 2024
tagliala
force-pushed
the
feature/267-remove-tsrange-override
branch
from
February 23, 2024 15:42
b6db434
to
0d559ef
Compare
orbanbotond
approved these changes
Feb 24, 2024
Time-zone awareness on
|
tagliala
force-pushed
the
feature/267-remove-tsrange-override
branch
from
February 26, 2024 11:33
0d559ef
to
061ebd2
Compare
This commit removes the custom `tsrange` parser implemented in Chronomodel in favor of the standard Rails `tsrange` parser. Implementation: - To maintain compatibility with Rails 7.0, continue returning `nil` instead of Infinity values, preserving the existing behavior of `valid_from` and `valid_to`. Ref: rails/rails#45099 - Bump major version because this is a potential breaking change Motivation: - Resolved dependencies: - Ruby 2.6+ now supports open-ended ranges, addressing the limitation in https://bugs.ruby-lang.org/issues/6864 - Rails `tsrange` stability issues in rails/rails#13793 and rails/rails#14010 - Standardized usage: - Aligns Chronomodel with Rails conventions, improving compatibility and reducing code complexity. - Allow to add `tsrange` to `ActiveRecord::Base.time_zone_aware_types` - Leverage official API: - Accesses potential future improvements and bug fixes in the standard `tsrange`. Considerations: - Applications relying on `validity.last` might require migration strategies due to open-ended ranges. Benefits: - Simplified codebase - Enhanced Rails compatibility - Future-proofed against potential API changes Close #269 Ref: - rails/rails#45099
tagliala
force-pushed
the
feature/267-remove-tsrange-override
branch
from
February 26, 2024 11:49
061ebd2
to
fffafa5
Compare
tagliala
added a commit
that referenced
this pull request
Feb 27, 2024
This method was used in features that were removed (#268 and #271) and the reason for converting ts values selected from the timeline query is not explicitly documented in the commit history and appears to be redundant (they are already timestamps) After conducting manual tests on the `usec`s, it was determined that the issue reported in issue #32 is no longer valid.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit removes the custom
tsrange
parser implemented inChronomodel in favor of the standard Rails
tsrange
parser.Implementation:
nil
instead of Infinity values, preserving the existing behavior of
valid_from
andvalid_to
. Ref: Support unbounded time ranges for PostgreSQL rails/rails#45099Motivation:
in https://bugs.ruby-lang.org/issues/6864
tsrange
stability issues in Dynamically define PostgreSQL Range OIDs rails/rails#13793 andNo longer map PostgreSQL ranges to Ruby ranges. Introduce a new PGRange type. rails/rails#14010
and reducing code complexity.
tsrange
toActiveRecord::Base.time_zone_aware_types
tsrange
.Considerations:
validity.last
might require migrationstrategies due to open-ended ranges.
Benefits:
Close #269
Ref: