Skip to content
This repository has been archived by the owner on May 28, 2022. It is now read-only.

Project Two Feedback #1

Closed
ajhofferber opened this issue Jan 7, 2016 · 4 comments
Closed

Project Two Feedback #1

ajhofferber opened this issue Jan 7, 2016 · 4 comments

Comments

@ajhofferber
Copy link
Collaborator

Hey Ben,

Nice job on this assignment. I really really liked your introduction to your project, where you talked about your own experience and how it informed your application, especially your choice to design the UI as mobile-first. Also, I thought the animation on your home page was a nice touch!

I noticed that you wrote your application helper to have a bunch of different methods, and then included it in your application controller so you could presumably have access to all of those methods throughout your application. I would suggest that for specific methods, like the currency conversion ones, you include them in the model for the table, or whichever model you are generating the receipt from. Only because it is generally suggested to try and modularize your code as much as possible.

It looks like you have a bunch of js files and a bunch of scss files that don't have anything in them? I'd suggest maybe cleaning up anything that is just blank, or there because you used rails generate. Also, you have your RSpec folder set up, but I am not seeing any tests written. Not sure if I am just missing them, but maybe think about adding some in there, it'll looks super impressive to people looking at your code!

Overall, I think you thought all of this out really well and should be proud of your finished project!

Score: 2 - Meets expectations, great!

@ajhofferber
Copy link
Collaborator Author

Hey Ben,

Just following up on my project feedback. So, upon further talk with instructors, because the inclusion of a test was listed as a requirement for this project, we cannot mark your project complete until we see a test included. If you get a chance this weekend, take some time and add it in! Then just let Avi and I know that it is there and we can make this as complete.

Sound cool?

@benbli
Copy link
Owner

benbli commented Jan 8, 2016

gotcha! Will do

On Fri, Jan 8, 2016 at 11:38 AM, ajhofferber notifications@github.com
wrote:

Hey Ben,

Just following up on my project feedback. So, upon further talk with
instructors, because the inclusion of a test was listed as a requirement
for this project, we cannot mark your project complete until we see a test
included. If you get a chance this weekend, take some time and add it in!
Then just let Avi and I know that it is there and we can make this as
complete.

Sound cool?


Reply to this email directly or view it on GitHub
#1 (comment).

@benbli
Copy link
Owner

benbli commented Jan 11, 2016

I added a test for favorite dish ordered at a table. It's in spec/helpers/application_helper_spec because I couldn't get it to run in it's own mode spec file for some reason. The method that I added is in the table.rb. please disregard the orderscontroller test. thanks!

48eceb4#diff-b979c2934ac0b4ba3f08dabfdd1b2299

@ajhofferber
Copy link
Collaborator Author

Great! Looks good! Thanks for taking the time to get that added. We've updated your project status to completed. 💯 👍 💯 👍 💯 👍 💯 👍 💯

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

No branches or pull requests

2 participants