manoa-inspire / MATP

MIT License
0 stars 0 forks source link

Review 10: SignInForm.jsx and SignUp.jsx #138

Open cjochim opened 1 week ago

cjochim commented 1 week ago

Overview

The focus for this code review will be centered around the SignInForm.jsx and SignUpForm.jsx page.

Please pay attention too:

Review Branch

review-10

Files to review

Checklists

Due date

11/18/24

For more information

The review process is documented at: http://courses.ics.hawaii.edu/ics414s21/morea/review/reading-idpm-review.html

cjochim commented 1 week ago

SignInForm.jsx: No deeply nested conditionals, no dead code, and readable code

SingUpForm.jsx Line 50: avoid using eslint-disable-next-line no-shadow Line 48: avoid using console.logs

Sydnee-You commented 1 week ago

SignInForm.jsx

Looks fine to me. Defaut export isn't renamed, if statements are not deeply nested, no ESLint errors, no long repetitions, etc.

SignUp.jsx

ES-01 and JS-06 on lines 48 and 52 because ESLint gave a warning for both lines due to them having console.logs.

RomaMalasarte commented 1 week ago

SignInForm.jsx:

L16: DE-02, Refactor handleSubmit to return early to avoid nesting

overall code is maintainable for now.

SignUp.jsx:

L51 & L57: DE-01, Refactor catch error blocks as they may be a duplicate

L48: JS-06, avoid using console.log

cash-baker commented 1 week ago

SignInForm.jsx

Good, readable code overall. Might want to include additional comments to explain certain code fragments.

SignUp.jsx

(ES-01) No errors, avoid warnings: Try to resolve ESLint warnings seen in L48, refrain from using console.log and using eslint-disable (L50) https://github.com/manoa-inspire/MATP/blob/4ae304e52413c31625e3741d3bc0685d8beedba5/app/imports/ui/pages/SignUp.jsx#L45-L50

blakewatanabe commented 6 days ago

SignInForm.jsx

DE-06: Ensure code is readable - The code is readable with no dead code or errors.

SignUpForm.jsx

JS-06: Avoid console.logs https://github.com/manoa-inspire/MATP/blob/4ae304e52413c31625e3741d3bc0685d8beedba5/app/imports/ui/pages/SignUp.jsx#L48-L52 ES-01: No errors, avoid warnings - If possible try and avoid using eslint-disable lines https://github.com/manoa-inspire/MATP/blob/4ae304e52413c31625e3741d3bc0685d8beedba5/app/imports/ui/pages/SignUp.jsx#L50