GitLab

@jacogr was reviewing the code in the beginning and provided useful input, but since he's not working on the codebase and it grew bigger, at one point it was becoming useless for him to review all PRs, esp. architecture-wise.

So I'm looking for volunteers to help me review the code before we ship Fether.

Guideline

• I put a list of files/folders in the table below, with some info about each item such as severity (security-wise) or difficulty of review (how easy would it be to review the item).
• You choose one or several items from the table, and for each item, complete the Review Template (inspired from Yelp).
• Create a reply in this github thread for each item you review, see Example. If you think the review doesn't pass and the code needs change, then open a separate Github issue.

Review Template

click to expand

Item: name of the item in the table

• Understand the code. Make sure you completely understand the code
• Check code quality. Check for well-organized and efficient core logic. Is the code as general as it needs to be, without being more general that it has to be? Make sure the code is maintainable.
• Verify that the code is tested well. Confirm adequate test coverage. Check tests having the right dependencies and are testing the right things.
• Check for any security holes and loop.
• Answer: If this code breaks at 3am and you’re called to diagnose and fix this issue, will you be able to?

Comments: comments if any

Example

click to expand (this should be one reply in this thread)

Item: myFile.js

• Understand the code.
• Check code quality.
• Verify that the code is tested well.
• Check for any security holes and loop.
• Answer: If this code breaks at 3am and you’re called to diagnose and fix this issue, will you be able to?

Comments: Missing some tests, and I think they should be required here. Will open a new github issue for this.

Review Items

Name	Description	Severity	Difficuty	Comments
MobX sendStore.js	Logic for sending tx	High	Easy	Make sure that we're sending the correct tx.
MobX parityStore.js	Require a secure token from the node	Medium	Medium	Make sure that this token is securely stored.
@parity/light.js	High-level Reactive JS lib (oo7-parity with RxJS)	Medium	Difficult	Can be skipped now, as it's a big codebase. Will create a similar issue as this one on that repo. It might make sense now to have a look at the Readme though.
@parity/light.js-react	Using parity/light.js with React (equivalent of oo7-react)	Medium	Medium	Uses JS Proxy.
MobX createAccountStore.js	Logic to create an account	Medium	Easy	Holds the user's secret phrase at some point.
fether-electron	Electron wrapper	Medium	Medium	Make sure Electron's guide is followed
@parity/electron	Interaction between Parity and Electron	Low	Medium
Other Mobx Stores	The rest of the MobX stores (other than the 3 above)	Low	Easy	Non-critical logic
React app	React app	Low	Easy	Contains minimal logic, only UI and UI state changes (all logic is in MobX stores)
fether-ui	UI Buildling Blocks Components	Low	Easy

It would be recommendable to get High- and Medium-severity items reviewed before shipping.