Skip to content

Conversation

@mldoscar
Copy link

No description provided.

@mldoscar
Copy link
Author

@guilleiguaran @benlangfeld Can you please take a look at this PR?

  • Added trilogy support without the need to depend on any mysql2 connections

  • Kept mysql2 support

  • Spec pass locally

  • Rubocop passing locally

@benlangfeld
Copy link
Member

@guilleiguaran @benlangfeld Can you please take a look at this PR?

  • Added trilogy support without the need to depend on any mysql2 connections

  • Kept mysql2 support

  • Spec pass locally

  • Rubocop passing locally

This is more up @xjunior 's street. He's been working on things related to this recently.

@mldoscar
Copy link
Author

mldoscar commented Feb 11, 2025

Thanks for the quick response. @xjunior I'll appreciate if you can take a look. This work was inspired on your previous PR, this PR makes trilogy to work as a standalone adapter without depending on mysql2 adapter

@douglas
Copy link

douglas commented Feb 18, 2025

Hello @mldoscar ! I just tried your branch on Rails 7.1 and I'm hitting this problem when running migrations: #110

Are you seeing the same there?

@mldoscar
Copy link
Author

mldoscar commented Mar 6, 2025

Hello @mldoscar ! I just tried your branch on Rails 7.1 and I'm hitting this problem when running migrations: #110

Are you seeing the same there?

You are right, I am seeing the same in rails 7.1. I'll dig more into it and I'll post a feedback in this thread. Might take some time but definitely this is something I'll have to take a deeper look at in a near future

@douglas
Copy link

douglas commented Mar 7, 2025

@mldoscar thanks a lot, I think your PR improved a lot of things in the codebase and if we could get that part to work it would be amazing.

Since there is no additional connection, perhaps we could actually make Departure a full-fledged AR adapter? This way, we don't need to care about changing adapters during migration and it should be closer to what AR expects since the change I mentioned in that thread.

In any case, count on me if you need any help 👍

@@ -1,5 +1,5 @@
FROM ruby:3.0
MAINTAINER [email protected]
# MAINTAINER [email protected]
Copy link

Choose a reason for hiding this comment

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

Is this any useful in this setup? We should probably get rid of this line.

@xjunior
Copy link

xjunior commented Mar 13, 2025

@mldoscar thanks a lot, I think your PR improved a lot of things in the codebase and if we could get that part to work it would be amazing.

Since there is no additional connection, perhaps we could actually make Departure a full-fledged AR adapter? This way, we don't need to care about changing adapters during migration and it should be closer to what AR expects since the change I mentioned in that thread.

In any case, count on me if you need any help 👍

Can you elaborate on how that would be possible?

@xjunior xjunior mentioned this pull request Mar 20, 2025
@xjunior
Copy link

xjunior commented Apr 4, 2025

@mldoscar are you still working on this? We merged rails 7.1 and 7.2 support to main, which included refactorings that conflicted with your branch, but should also contribute to your solution.

We're excited to support Trilogy, so please let us know if you're not going to be working on this.

@mldoscar
Copy link
Author

mldoscar commented Apr 4, 2025

@xjunior I can try to rebase my branch and work on the trilogy compatibility based on the recent changes in the master branch at some point

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.

4 participants