-
Notifications
You must be signed in to change notification settings - Fork 40
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
Wade branch #16
base: master
Are you sure you want to change the base?
Wade branch #16
Conversation
Added two more commits for the other parts. |
ActiveRecord::Schema.define(version: 0) do | ||
|
||
create_table "urls", force: true do |t| | ||
t.string "linkid", limit: UrlsHelper::LINKID_LENGTH, null: false |
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.
Was this created by a migration? If not, please read the comment at the top of this file for some guidance.
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 normally don't use code generators/wizards until I understand them. I spent a fair amount of time working to understand this and then did not go back to the migrations.
I will correct that in a new pull request.
Instead of creating by hand, I used rails generate to build the initial model.
After a challenge from Mike on the last version, I tried to do this without building a whole, separate validation class. I couldn't get message to work on the 'presence' validation, so I did a separate validation function for it. I could have combined all of these into one large validation function, but I decided it was more interesting to make a number of simple validation functions instead.
Filled out the tests for the model and controllers. Still trying to get used to using the generator. |
@errors = nil | ||
|
||
@url = Url.new(target_link: @target) | ||
if [email protected] |
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 prefer the happy path as the positive part of the if
and the error path under else
.
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.
Fair enough.
* if/else handling of error * Use of .blank? * Correct start of string anchor
Two steps into this challenge, one commit per release.
Please review @chivygab @jaybobo @mikegee