Skip to content
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

Yordanos Dirar - Task List #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

yordanosd
Copy link

https://yd-task-list.herokuapp.com/

Please note, some links are not working. Sorry about the mess.

Yordanos

redirect_to root_path
end

def save_edit
Copy link

@loganmeetsworld loganmeetsworld Apr 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does save_edit do? Not immediately apparent.

@loganmeetsworld
Copy link

loganmeetsworld commented Apr 26, 2016

Yordanos, this looks great! Amazing that you've been doing Rails for a week and able to do this!

My only big feedback is that you should commit more often!!!! And write descriptive commit messages. I never built up this habit in Ada and now am kicking myself at my job. It's really important to commit when you make any change and describe what it is you did. I would say as a rule, if you ever find yourself typing git add . don't! Add one file or one folder at a time and comment on what you're doing in that commit like git add spec/, for example. git add . will also be a sure way to add bad stuff like security information or keys down the line...

Other than that, great style and great Rails app! Your logic at the controller level is sound and the CSS is awesome. If you have any questions about Rails/my comments please reach out to me. I'm always available. 👍 💯 Logan

# # (app/controllers/admin/products_controller.rb)
# resources :products
# end
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to remove all this extra commented out stuff Rails gives you, but you don't have to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants