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

Add active_record.postgresql_adapter_decode_dates config #51763

Merged

Conversation

JoeDupuis
Copy link
Contributor

Add active_record.postgresql_adapter_decode_dates config
to toggle automatic decoding of dates column with the PostgresqlAdapter.

PR #51483 is a breaking change and should have been gated behind a config.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@JoeDupuis JoeDupuis force-pushed the postgresql-adapter-decode-dates-config branch 2 times, most recently from fddc0b8 to b4fcbe7 Compare May 8, 2024 23:43
@JoeDupuis JoeDupuis requested a review from ghiculescu May 8, 2024 23:45
@JoeDupuis JoeDupuis force-pushed the postgresql-adapter-decode-dates-config branch from b4fcbe7 to dafe0fc Compare May 9, 2024 00:19
@JoeDupuis JoeDupuis force-pushed the postgresql-adapter-decode-dates-config branch 2 times, most recently from b2bfd24 to 153fbaf Compare May 9, 2024 16:48
@ghiculescu ghiculescu requested a review from byroot May 9, 2024 22:52
@byroot byroot force-pushed the postgresql-adapter-decode-dates-config branch from 153fbaf to 401c1d3 Compare May 10, 2024 08:29
@JoeDupuis JoeDupuis force-pushed the postgresql-adapter-decode-dates-config branch from 401c1d3 to c40b8b7 Compare May 10, 2024 23:31
@JoeDupuis
Copy link
Contributor Author

JoeDupuis commented May 13, 2024

@byroot I rebased to fix the failing tests.
Looks like there is still a flake even if Github is showing that the test suite passed.

I pushed a attempt at a fix for the flake: #51783

@JoeDupuis JoeDupuis changed the base branch from main to 7-2-stable May 15, 2024 16:26
@JoeDupuis JoeDupuis force-pushed the postgresql-adapter-decode-dates-config branch from c40b8b7 to 95f9822 Compare May 15, 2024 16:26
@skipkayhil skipkayhil added this to the 7.2.0 milestone May 15, 2024
@rafaelfranca rafaelfranca changed the base branch from 7-2-stable to main May 15, 2024 18:36
to toggle automatic decoding of dates column with the
PostgresqlAdapter.

PR rails#51483 is a breaking change and should have been gated behind a
config.
@rafaelfranca rafaelfranca force-pushed the postgresql-adapter-decode-dates-config branch from 95f9822 to 478874a Compare May 15, 2024 18:38
@rafaelfranca rafaelfranca merged commit a82e432 into rails:main May 15, 2024
3 checks passed
rafaelfranca added a commit that referenced this pull request May 15, 2024
…tes-config

Add `active_record.postgresql_adapter_decode_dates` config
@zzak
Copy link
Member

zzak commented May 18, 2024

If you're using AR outside of Rails, this now stops decoding dates entirely, right?

@JoeDupuis
Copy link
Contributor Author

@zzak Yup, without Rails, it would default to the old behavior before the breaking change was introduced.

I am hoping to add a deprecation warning at some point and remove the old behavior entirely. Adding the deprecation warning in this PR turned out more complex than anticipated.

Any concern?

@zzak
Copy link
Member

zzak commented May 19, 2024

Nope, sorry I am still catching up. Thank you!

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

Successfully merging this pull request may close these issues.

None yet

5 participants