-
Notifications
You must be signed in to change notification settings - Fork 209
Add mongoid support #99
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
Conversation
👍 |
* Code adapted from https://github.com/bowsersenior/workflow_on_mongoid * Update adapter activation code to handle the case when more than one ORM class is defined * Add tests to ensure everything works with Mongoid adapter * Update gemspec to include mongoid * Update .travis.yml to add mongodb service for mongoid tests * Tweak gemspec and gemfiles/Gemfile.rails-* * ignore .lock files in gemfiles folder
I know, mongodb is pretty popular nowadays. So there is a good reason to support it. On the other side including a mongodb adapter also means additional maintenance effort (even if workflow_on_mongoid contains an extended test suit) and maintenance obligation - once included into workflow library, if new issues with mongodb integration appear, that I can not debug, I can not just kick the adapter out again. So for me the next step would be to check, why it so painful to keep up with workflow and maintain a separate adapter. Versioning schema? Incompatible API changes? Will have a careful look at the conversation bowsersenior/workflow_on_mongoid#12 and think about improvement. |
Hi @geekq , thanks for the comments. I understand your position. The difficulty in adding an adapter to workflow is that I would have to monkeypatch the main |
Hi Mani, if we are talking about Mongo support in workflow, we are talking only about persistence of the
If so, there is no need for monkeypatching. We can squash this two additional lines into one and advise the user of
In the workflow_on_mongoid the include-glue can be implemented roughly as:
Can you try this out? If it does not work, I'll find time to fork |
Hi, I tried that out initially but there are problems and edge cases, such as when both Anyways, it's clear you are hesitant to include this patch in workflow. Mongoid support does take a little extra support, but if you look at my pull request, I've already done the work for you. I don't have the time to support workflow_on_mongoid any longer, so I'll just update the README to point people to alternative state machines like https://github.com/pluginaweek/state_machine which support Mongoid out of the box. Thanks for your feedback. |
Hi Mani, if you do not have time/energy to support workflow-mongo, how should I take over the Mongo support? (integrate it into the core of workflow) I do not even use Mongo currently!
What you can also write, is if somebody wants to work on |
This came up a few years ago (see #16 and #18) but I thought I'd try again as I see workflow now has separate adapter classes, which makes it a bit easier to add new adapters.
It's actually a bit of a pain to keep workflow_on_mongoid up to date with changes in workflow (see bowsersenior/workflow_on_mongoid#12), so I thought it would be logical to add the small
Mongoid
adapter to workflow itself.Let me know your thoughts. Thanks!