-
Notifications
You must be signed in to change notification settings - Fork 21
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
some code, typos and consistency fixes #26
base: master
Are you sure you want to change the base?
some code, typos and consistency fixes #26
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for your diligent feedback :) I've got a few comments and then I think this will be good to merge.
@@ -217,12 +217,12 @@ Then the migration has been successfully applied. | |||
|
|||
In order to get data into and out of database tables with ROM, we need to create something called a _repository_. A repository is a class that is used to define a clear API between your database and your application. | |||
|
|||
To create one of these, we'll create a new file inside a new directory structure at `lib/bix/repos/user_repo.rb`: | |||
To create one of these, we'll create a new file inside a new directory structure at `lib/bix/repos/user.rb`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to leave this as user_repo
because later on when we import it with dry-auto_inject
it'll make it available as a variable called user_repo
, not user
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing!, in the code samples it's mostly Bix::Repos::User
though, also in part 2 (haven't been up to part 3 ;). Will have to update the code samples to be Bix::Repos::UserRepo
as well then.
@@ -234,12 +234,12 @@ To use this class (and others that we will create later on), we'll need to creat | |||
```ruby | |||
Bix::Application.boot(:persistence) do |app| | |||
start do | |||
register('container', ROM.container(:sql, app['db.connection'])) | |||
register('container', ROM.container(:sql, app['db.config'].gateways[:default].connection)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right that this line needs changing, but I just figured out you can do this:
register('container', ROM.container(app['db.config']))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was my first attempt as well, it did not work for me. The db.config returns a ROM::Configuration instance, where the container expects a database connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, there's a bit more that needs to change. I've revised Part 1 just this morning and the relevant changes for this part are: 57aae3a#diff-0de0c2c310f3cf59a6ea6fdc92b1af3aR368
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome! I finally got to the point where I actually understand the need for user_repo.rb
and create_user.rb
. The dependency injection makes it so much more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's so great! :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
figured out we can use aliases! Love it even more.. Import[user_repo: 'repos.user']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh great, I didn't know about those aliases.
@@ -254,7 +254,7 @@ require 'irb' | |||
IRB.start | |||
``` | |||
|
|||
This file will load our application's `config/application.rb` file. When this file is loaded, all the files in `lib` will be required. This includes our new `lib/bix/repos/user_repo.rb` file. | |||
This file will load our application's `config/application.rb` file. When this file is loaded, all the files in `lib` will be required. This includes our new `lib/bix/repos/user.rb` file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep this as user_repo.rb
@@ -363,7 +363,7 @@ However, we will need to register relations with our application's database cont | |||
```ruby | |||
Bix::Application.boot(:persistence) do |app| | |||
start do | |||
container = ROM.container(:sql, app['db.connection']) do |config| | |||
container = ROM.container(:sql, app['db.config'].gateways[:default].connection) do |config| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be:
ROM.container(app['db.config'])
@@ -381,12 +381,12 @@ Let's run `bin/console` again and try working with our repository again: | |||
NoMethodError (undefined method `all' for #<Bix::Repos::User struct_namespace=ROM::Struct auto_struct=true>) | |||
``` | |||
|
|||
Oops! Repositores are intentionally bare-bones in ROM; they do not come with very many methods at all. Let's exit the console and then we'll define some methods on our repository. While we're here, we'll add a method for finding all the users, and one for creating users. Let's open `lib/bix/repos/user_repo.rb` and add these methods: | |||
Oops! Repositories are intentionally bare-bones in ROM; they do not come with very many methods at all. Let's exit the console and then we'll define some methods on our repository. While we're here, we'll add a method for finding all the users, and one for creating users. Let's open `lib/bix/repos/user.rb` and add these methods: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep this as user_repo.rb
.
|
||
```ruby | ||
module Bix | ||
module Repos | ||
class UserRepo < ROM::Repository[:users] | ||
class User < ROM::Repository[:users] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep this as UserRepo
@@ -427,7 +427,7 @@ Hooray! We have now been able to add a record and retrieve it. We have now set u | |||
* `system/boot/db.rb` - Specifies how our application connects to a database | |||
* `system/boot/persistence.rb` - Defines a ROM container that defines how the ROM pieces of our application connect to and interact with our database | |||
* `lib/bix/relations/users.rb` - Defines a class that can contain query logic for our `users` table | |||
* `lib/bix/repositories/user.rb` - A class that contains methods for interacting with our relation, allowing us to create + retrieve data from the databse. | |||
* `lib/bix/repos/user.rb` - A class that contains methods for interacting with our relation, allowing us to create + retrieve data from the database. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please switch this to user_repo.rb
.
@@ -449,12 +449,12 @@ end | |||
|
|||
Ignoring [the falsehoods programmers believe about names](https://www.kalzumeus.com/2010/06/17/falsehoods-programmers-believe-about-names/), this method will combine a user's `first_name` and `last_name` attributes. | |||
|
|||
To use this class though, we need to configure the repository further over in `lib/bix/repos/user_repo.rb`: | |||
To use this class though, we need to configure the repository further over in `lib/bix/repos/user.rb`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep this as user_repo.rb.
|
||
```ruby | ||
module Bix | ||
module Repos | ||
class UserRepo < ROM::Repository[:users] | ||
class User < ROM::Repository[:users] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep this as UserRepo
.
@@ -519,12 +519,12 @@ module Bix | |||
end | |||
``` | |||
|
|||
This `Import` constant will allow us to import (or _inject_) anything registered with our application into other parts. Let's see this in action now by adding this line to `lib/repos/user_repo.rb`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep this as user_repo.rb
Please pull down latest master and resolve conflicts. |
While going through this awesome step by step post, I ran into some issues. This commit is an attempt to give back by updating this post with a couple of fixes.
4bd09e8
to
523c198
Compare
will do second pass in my am. Some |
While going through this awesome step by step post, I ran into some issues. This commit is an attempt to give back by updating this post with a couple of fixes.