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.

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

Time: fix TZ offset #5586

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@dr-itz
Copy link
Contributor

dr-itz commented Jan 31, 2019

The setTzRelative flag wasn't set everywhere where it should. For one,
the 9.2 changes to initTime() broke Rails' Time.change(). While this
would have been fixable by setting the flag as well, it's just way
too fragile. When a new Time is created based on an instance with
the flag set, it was simply lost. Just returning nil instead of ""
in zone() would fix the basic Rails test, but thing like inspect on
a cloned or otherwise copied-and-adjusted object would still break...

So instead, just remove the flag altogether and evaluate where needed.
This does indeed fix the Rails case, verified this time.

See also jruby/activerecord-jdbc-adapter#980

Time: fix TZ offset
The setTzRelative flag wasn't set everywhere where it should. For one,
the 9.2 changes to initTime() broke Rails' Time.change(). While this
would have been fixable by setting the flag as well, it's just way
too fragile. When a new Time is created based on an instance with
the flag set, it was simply lost. Just returning nil instead of ""
in zone() would fix the basic Rails test, but thing like inspect on
a cloned or otherwise copied-and-adjusted object would still break...

So instead, just remove the flag altogether and evaluate where needed.
This does indeed fix the Rails case, verified this time.
@dr-itz

This comment has been minimized.

Copy link
Contributor Author

dr-itz commented Jan 31, 2019

So...this breaks a few formatting tests...and the absolute perfect thing is that at least 'test:mri:stdlib' and 'spec:ruby:fast' pass locally...
guess I'll just go for the nil-vs-empty-string thing

@dr-itz dr-itz closed this Jan 31, 2019

@enebo enebo added this to the JRuby 9.2.6.0 milestone Feb 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.