Add Sequel preliminary support

[snacks02]

Thanks for the amazing library!

Fixes #28.

What is the purpose of this pull request?

This is an attempt to get the original Sequel preliminary support PR merged.

What changes did you make? (overview)

Merged master from upstream, fixed conflicts, took the comments from the original PR into account.

Is there anything you'd like reviewers to focus on?

The original PR is a brilliant piece of work, so I just wanted it merged so I could use upstream Logidze for a project. Not sure if there's anything else to say. :)

Checklist

• I've added tests for this change
• I've added a Changelog entry
• I've updated a documentation (Readme)

^ All of this was already done.

[ardecvz]

@sanks02 Hello, thank you for your work and for advancing the agenda!

This feature is indeed important, especially since there isn't a record tracker within the Sequel ecosystem.
We should certainly make a final push to make it into the gem.

There are just 2 things left to address:

1. We overlooked this comment - Add Sequel preliminary support #229 (comment)
   The term "adapter" is a bit too generic. Considering we might support other databases like MySQL in the near future, we should rename it.
2. It might be a good idea to remove the strict dependency on ActiveSupport before the release (Add Sequel preliminary support #229 (comment)).
   We only employ it for JSON serialization / deserialization as seen here:
   https://github.com/palkan/logidze/pull/242/files#diff-fbdceda80b1b58da4bd608bbe7ee747e0d3569ecc991fbfcbd6688c02fc793ecR12-R32

The required steps would be:

1. Use the JSON encoding / decoding features provided by Sequel:
   https://github.com/jeremyevans/sequel/blob/master/lib/sequel/core.rb#L169-L177
2. Run tests and set up two demo apps, one with ActiveRecord and the other with Sequel only. So we can ensure nothing is broken without ActiveSupport.

@sanks02, would you be interested to help with these tasks?
It would be a great help (also do not forget to add yourself to the contributors list in the README.md).

If you're busy, I'll hope to address these issues this week.

[snacks02]

@ardecvz Thanks for the heads up! It doesn't sound complicated, so I think I can do it.

[snacks02]

I'm halfway through adding bundle exec rake spec:sequel that doesn't depend on ActiveRecord and ActiveSupport.

There is a problem with generators depending on ActiveRecord. I'm not sure how to remove this dependency, so I'll remove Sequel-specific code from generators and associated specs for now.

Later we can see what sequel-rails does, and try to do a similar thing to add the generators back. Does this sound good?

[snacks02]

@ardecvz Would you mind helping me from this point?

[ardecvz]

Hi again!
Sorry for the late reply, I missed the first notification.

Oh, you've found an interesting edge case! And we're so close to the proper merge.

According to Sequel's philosophy, you don't need generators - the authors recommend using your normal text editor environment.

So, your recent removal of generators is fine, but in this case, we need to prepare a separate, detailed guide on how to craft installation and model migrations as done in Sequel.

However, in my opinion, Logidze migrations are much harder to copy and paste manually without mistakes.
And Evil Martians' gems are all about quick starts and usability, so I personally recommend keeping them.

Yes, we'll keep a dependency on Railties (and ActionPack, unfortunately), but we'll remove the redundant dependency on ActiveRecord.

On the other hand, Sequel is popular among minimal environments, so ActionPack may be overkill for them.

Also, I'm not aware of any lightweight alternatives to Rails generators (sequel-rails seems to rely on Railties).

It's a decision with drawbacks, so let's get @palkan to decide:

1. Keep a dependency on Rails generators, accepting the extra ActionPack from Railties;
2. Find a lightweight alternative to generators, potentially compromising the main gem's codebase;
3. Remove generators for Sequel and create a detailed guide as intended by Sequel.

[palkan]

I'm not aware of any lightweight alternatives to Rails generators

We can use plain Thor; that would, probably, require adding a standalone bin/logidze CLI to perform the installation, etc.

Remove generators for Sequel and create a detailed guide as intended by Sequel.

Another option is to add an application template via Ruby Bytes: https://github.com/palkan/rbytes

Basically, the same idea as using Thor but without dealing with CLIs, etc.

[snacks02]

It looks like Ruby Bytes does not support RSpec, which is used in this project.

[snacks02]

@ardecvz I added the Sequel generators back.

My knowledge of Rails generators, Thor and Ruby Bytes is limited, so if you don't mind picking the PR up, I'd appreciate it.

The issues with the current generators are:

• logidze/lib/generators/logidze/install/install_generator.rb

  Lines 95 to 97 in caf7d90

  	def self.next_migration_number(dir)
  	::ActiveRecord::Generators::Base.next_migration_number(dir)
  	end

• logidze/lib/generators/logidze/model/model_generator.rb

  Line 10 in caf7d90

  class ModelGenerator < ::ActiveRecord::Generators::Base # :nodoc:

[snacks02]

It would be great to have this PR merged.

I think the easiest solution is to add the ActiveRecord/ActiveSupport dependency back (but keep all other fixes and mention the ActiveRecord/ActiveSupport dependency in README.md).

Or do you have any other suggestions/recommendations?

[ardecvz]

Yes, I'm with your plan - adding support gradually is the only path forward right now.
Have we test the new version in both the ActiveRecord and Sequel dummy applications?
If that is the case, I believe @palkan will cut a release shortly after the merge.

[ardecvz]

And we should create an issue with future improvements - to keep a plan forward.

[snacks02]

Thanks. The branch is in a half-baked state right now. I'll fix it and let you know when it can be reviewed.

[snacks02]

Should be ready for a review.

Suggestions from the original PR have been taken into account and small changes have been made to further separate Logidze from ActiveSupport.

I didn't finish adding a separate Sequel demo as it would be impossible to test if it works without ActiveRecord anyway, but I mentioned it in the newly created issue.

The difference between this and the original PR. I hope this helps. Let me know if anything else is needed. :)

[palkan]

Thanks @sanks02!

Please, take a look at the RuboCop offenses (look legit) and RSpec failures.

@ardecvz Waiting for your approval 🙂 (and the green CI, of course)

[snacks02]

Sorry about that, I may have accidentally tested bundle exec rake on a wrong branch. Should be fixed now.

[snacks02]

diff.tar.gz

[snacks02]

I've started using this in a real world application and it seems to work just fine

[palkan]

@sanks02 Thanks!

@ardecvz I'm still waiting for your pair of eyes 👀