-
Notifications
You must be signed in to change notification settings - Fork 9
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
bEtsy by Bestie - Lisa, Jillian, Justine, Melissa #1
base: master
Are you sure you want to change the base?
Conversation
…e, #edit and private method products params to the nested controller
…ce in dollars (with or without dollar sign) and return an integer representing price in cents
…x and show pages, and added require_login method to application_controller
…ute to the nested controller
…e out whether to make a separate Category model
… hitting submit (these forms are missing a category option currently, but are otherwise functional without it)
…at users full list of products
…_controller (removed require_login from application_controller until someone else needs it in a different controller
… owner of the current products page.
…oduct page, otherwise it redirects you to your own page and gives an error that you don't have access to the previous account
…t, or 'no reviews yet.'
Merge branch 'master' of github.com:Lisa-Sano/betsy
<h2>Total: <%= to_money(@order.order_total) %></h2> | ||
<% end %> | ||
|
||
</div> |
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.
Yay Partials!
Wow! How do I summarize such an amazing project? Great job everyone! Reminder of my deficiencies. If you want detailed feedback on TestUnit, Javascript or CSS, you will want to ping someone with more experience in these areas. One of the great things about the code review process though is that you'll likely get a different person each time so maybe next time you'll get someone with more experience in one of those areas. This is a very big project. Getting something working and working well is very challenging and working in a team of 4 is also challenging. Great job rising to the occasion. I have a lot of comments here but you shouldn't in any way take it as criticism of what you have accomplished. Many of my comments are things that will move you forward as developers but are not things that kept you from creating a great product. The first step is always to get something working and you all have achieved that very well. As a general statement it seems as though testing is something that would be a great next place to focus on for all the team members. Being able to test well really provides you peace of mind when you are coding and especially when you are refactoring. Having solid testing skills will serve you well in your career. Team communication would also be something to work on as well. Reading each other's commits and doing mini-code-reviews for each other will help everyone understand the code base well. There are a couple places where there were helpful abstractions that were either not universally implemented because those parts of the code were unknown to the person doing the abstraction, or were not used by the person who could have used it when creating another place that used that or a similar abstraction. Regular team communication, particularly more pairing time and code reviews will also help have another pair of eyes when naming things. Naming things is one of the hard problems in tech so getting more eyes on the problem and talking it out with your teammates can really help you find some clear naming conventions that makes the code more maintainable in general. There are lots of comments in there. I recommend you read through them but don't stress out about them. Like I said you all did wonderfully. If you feel inspired to read an article or implement a fix, go for it. But I understand that Ada is a constant marathon that doesn't really let up. So, read the comments but don't feel any pressure to do anything about them. Just see if some of those comments can seep into your brains a bit to help you when you run into similar problems next time. Great job! I look forward to seeing what you all do next! |
Oh, One more comment. |
<%= f.button ("Create!"), class: "btn btn-success" %> | ||
</span> | ||
</div><!-- /input-group --> | ||
</div><!-- /.col-lg-6 --> |
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 always best practice to go through your code and remove and unnecessary comments
betsybybestie.herokuapp.com
https://trello.com/b/BrolWWTj/betsy-by-bestie