-
Notifications
You must be signed in to change notification settings - Fork 240
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
added neo4j adapter #128
base: master
Are you sure you want to change the base?
added neo4j adapter #128
Conversation
Hi @pdipietro, The Neo4j support addition is coordinated from #98, please be sure to comment about your PR there so we stay in sync. (Let's comment here about specificities of you PR and there about the Neo4j support.) A few comments about your PR: First of all, thank you! As mentionned in the README, tests can't be missing. They ensure each new feature is both robust and documented. The adapters interface allows to extends support for ORM/ODM/OxM in a standard way (see #97 please, the discussion documents the intentions behind the code). I documented that standard way while adding support for the Mongoid ODM, and you can find it described step by step in #124. My intention was making the protocol easy to follow, and hopefully easy to understand even if you're not familiar with test-driven development. Don't hesitate to ask if something is unclear. : ) The last one is a detail: I use to enforce some naming conventions for branches to keep the repository graph readable, and I would suggest this one to be named Finally, I've never seen your error. I can't check the Travis build from my phone, but I |
(...continuing) I'm pretty sure that only the test which documents the default |
Looks good, you should change the name back in the Gemspec to gonzalo-bulnes, though. |
@ianks: done! |
Some more notes:
|
Hi all,
after signaling an error on the wiki, I decided to add a new adapter.
Here you can find the code, BUT there is still the unresolved error I signaled in the Issue #127:
pdipietro@CUC-N4J-V4-0-0:~/gsn$ rails s
simple_token_authentication/adapters/active_record_adapter
simple_token_authentication/adapters/mongoid_adapter
simple_token_authentication/adapters/neo4j_adapter
bin/rails:6: warning: already initialized constant APP_PATH
/home/pdipietro/gsn/bin/rails:6: warning: previous definition of APP_PATH was here
Usage: rails COMMAND [ARGS]
Please, look at the error and then decide if you can merge this fork