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

Betsy - Team MAGS - Braineaze - Amber Lynn - Goeun - Maddie - Semret - EDGES #19

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

Conversation

snicodimos
Copy link

bEtsy

Congratulations! You're submitting your assignment! These comprehension questions should be answered by all members of your team, not by a single teammate.

Comprehension Questions

Question Answer
Each team member: what is one thing you were primarily responsible for that you're proud of? Amber-Lynn: HTML/CSS, order-item model testing, review model testing, Semret: Seeding and Session testing, Goeun: Category / review / merchant controller testing, Maddie: Order controller methods and testing
Each team member: what is one thing you were primarily responsible for that you would like targeted feedback on? Amber-Lynn: HTML/CSS/Bootstrap, Semret: Testing and capturing edge cases, Goeun: Routes, params, forms in views, Maddie: One thing I would like feedback on would be when to render vs redirect as I realized rendering would still go to the next page at times
How did your team break up the work to be done? We utilized trello heavily. The first week we worked together to make sure set up done properly (set up routes, models and controller) with TDD.
How did your team utilize git to collaborate? Our git hygiene was good but we should have been more cognizant of letting others know when we pushed (especially after hours) and pushing broken code. We also utilized branches.
What did your group do to try to keep your code DRY while many people collaborated on it? We used partials in product view and helper methods in models and controllers. We revisited our code days later or had a fresh pair of eyes look at them to refactor.
What was a technical challenge that you faced as a group? We didn’t set up views early enough because we were focused on TDD and didn’t deploy as early as planned. Next time, we would have bare-bones views set up initially.
What was a team/personal challenge that you faced as a group? We had a 45 minute discussion on this but generally: communication, boundaries, imposter syndrome, better organization at the beginning!
What was your application's ERD? (include a link) https://www.lucidchart.com/invitations/accept/977b972c-d24f-4d77-87df-6a499f73e447
What is your Trello URL? https://trello.com/b/avRZAxjD
What is the Heroku URL of your deployed application? http://braineaze.herokuapp.com/

snicodimos and others added 30 commits October 22, 2018 17:09

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@snicodimos snicodimos changed the title Betsy - Team MAGS - Braineaze - Amber Lynn - Goeun - Maddie - Semret Betsy - Team MAGS - Braineaze - Amber Lynn - Goeun - Maddie - Semret - EDGES Oct 26, 2018
@droberts-sea
Copy link

bEtsy

Requested Feedback:

Amber-Lynn: HTML/CSS/Bootstrap

  • HTML is well-organized, semantic HTML is well-used
  • CSS feels well-organized, I don't see any clear repetition, but CSS is also really hard to just sit down and read.

Semret: Testing and capturing edge cases

"testing" is not "targeted feedback", but I looked at a couple controller actions:

  • OrderItems#create This is solid coverage! I only see one missing case: what if you add a product that's already in the cart, and the amount you add would put it over the product's total inventory. Also, this logic is pretty complex, is there a way you could move it to the model? Maybe making it an instance method on Order.
  • Products#update You need some tests for authorization here. What if the user is not logged in? What if they're logged in as someone other than the merchant who owns this product?
  • Reviews#create Good work, especially around authorization testing

Goeun: Routes, params, forms in views

  • I left some inline feedback on routes. Is there a specific place where params or a form gave you trouble?

Maddie: One thing I would like feedback on would be when to render vs redirect as I realized rendering would still go to the next page at times

  • I don't see anywhere these are obviously misused. Do you have a specific place in mind?

Only the person who submitted the PR will get an email about this feedback. Please let the rest of your team know about it.

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.

None yet

5 participants