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

Tommy Takahashi and Sam Tran #93

Open
wants to merge 86 commits into
base: main
Choose a base branch
from

Conversation

Sykogst
Copy link

@Sykogst Sykogst commented Dec 2, 2023

We think the project went well and it was fun working with each other. We are proud of learning how to do API consumption and refactoring it using the service/facade/poro organization structure.

We would like feedback specifically on a couple of things.

  1. Polymorphism, as we were not fully sure on how this is implemented using the service/facade/poro organization. We had an idea on how it worked as presented in the vide. Some feedback on if we actually did use it in any of our code, or how we could implement it if it was missing would be awesome.
  2. Refactoring and better implementation of HTML/CSS in the view, we know that there are some tools/gems we will use later on for view formatting, but they can get potentially cumbersome. What are some low-hanging items of refactor or types of HTML that can be used within the rails app itself?

ttakahashi1591 and others added 30 commits November 27, 2023 16:35
… show page redirecting to the User Discover Index page
Sykogst and others added 30 commits December 1, 2023 14:59
…ent for readability, remove handrolled route
Thanks for taking care of these refactors and updates!
…on_happy_path

Register With Authentication Happy Path
…cation_sad_path

Registration (w/ Authentication) Sad Path
…er_method

Add Sessions And Memoization Of Current User
Task 4: User Story 2 & Task 5: User Story 3
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