Review: QRCodeGenerator and OrgScanQR

[thomasarivera]

Overview

The focus for this code review will be centered around the QRCodeGenerator component and the OrgScanQR page.
Please pay attention too:

• Javascript issues
• React components

Review Branch

review-162

Files to review

• QRCodeGenerator.jsx
• OrgScanQR.jsx

Checklists

• Architecture
• Design
• JS
• React
• Testing

Due date

Monday, April 15 11pm

For more information

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

[ryanseng03]

QRCodeGenerator.jsx

• Could add a few comments
• Looks good overall

OrgScanQR.jsx

• Looks good

[Jchen20-1]

QRCodeGenerator.jsx

Looks good

OrgScanQR.jsx

Looks good

[willjsimmons]

QRCodeGenerator.jsx

---

It looks good to me, and is very impressive. A few comments may be helpful.

OrgScanQR.jsx

---

looks good

[jianleliu]

QRCodeGenerator.jsx

• looks good

OrgScanQR.jsx

• L33: use subscribeSingleEvent(eventid) instead of subscribeEvent() for better performance.
• L61: similarly, you can subscribe to a single organization instead of all of them.

[aldenparoni]

QRCodeGenerator.jsx

Looks good

OrgScanQR.jsx

L74: warning, lint says "variable initializer is redundant" L120: will relevant user information be added after this comment? if not then delete

[thomasarivera]

QRCodeGenerator.jsx

TA-01:Each Page has an isDisplayed() acceptance test DE-05: Ensure comments are appropriate. Add comments for clarity

OrgScanQR.jsx

TA-01:Each Page has an isDisplayed() acceptance test

[sierranmorales]

QRCodeGenerator.jsx

Add comments for better readability

OrgScanQR.jsx

L74 "Variable initializer is redundant"

[hokwaichan]

QRCodeGenerator.jsx Can add comments for clarity

OrgScanQR.jsx Looks good

[thomasarivera]

Outcome of meeting:

Changes made in review branch and pushed to main:

QRCodeGenerator.jsx

Implement comments for code readability.

OrgScanQR.jsx

Implement better subscriptions

New issues:

Acceptance tests for QRCodeGenerator and OrgScanQR (will be resolved in issue-163)