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

Review: First Review #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

leafoflegend
Copy link

Theres a lot of non-sanctioned-fullstack stuff happening here. Which, yeah, I think its dope. But it does at times feel like you guys are fumbling about a bit in the dark.

  1. You absolutely must remove your credentials from a non-hidden file.
  2. Your use of redux is confused and all over the place.
  3. There is no backend? Is this the intention with using firebase? Thats fine - but it seems unclear.

I think I just need some larger answers about direction you guys are looking to go here. But regardless, TODO for next Saturday:

  1. Fix leaky secrets.
  2. Fix all comments in this PR.
  3. Build out models, or have firebase up and fully functioning.
  4. I'd like to see a front end for the cart.

@tflesui
Copy link
Contributor

tflesui commented Jul 24, 2021

Credentials have now been moved to a .env file

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

2 participants