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

Avoid referencing the AR connection adapter in the toplevel #42

Merged
merged 1 commit into from
Sep 27, 2019
Merged

Avoid referencing the AR connection adapter in the toplevel #42

merged 1 commit into from
Sep 27, 2019

Conversation

amatsuda
Copy link
Contributor

@amatsuda amatsuda commented Jun 5, 2019

This PR fixes #41 not by documenting but by fixing the code.

This implementation expects AR postgresql connection adapter to be already loaded while loading this gem, but this assumption is not always true because AR usually loads the adapters after loading config/database.yml and datermining the database adapter to load.

Also, defining a global constant like this OID thing inside a gem is pretty rude. It may conflict with the users' application code or other gems.

This new constant has been introduced via #35 just for resolving an ambiguous OID constant refernce, but that ambiguity here could be more simply and naturally solved just by referencing the constant with the absolute path per each AR version.

And, we'd better not define a toplevel constant for this

fixes #41
@IlyaKamenkoTrycarriage
Copy link

👍

1 similar comment
@johvet
Copy link

johvet commented Aug 18, 2019

👍

@afair afair merged commit 024d450 into afair:master Sep 27, 2019
@amatsuda amatsuda deleted the no_toplevel_oid branch September 27, 2019 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

uninitialized constant ActiveRecord::ConnectionAdapters::PostgreSQLAdapter
4 participants