Skip to content

GitLab

  • Menu
Projects Groups Snippets
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
  • Sign in
  • F fether
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Locked Files
  • Issues 20
    • Issues 20
    • List
    • Boards
    • Service Desk
    • Milestones
    • Iterations
    • Requirements
  • Merge requests 1
    • Merge requests 1
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
    • Test Cases
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages & Registries
    • Packages & Registries
    • Container Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Code review
    • Insights
    • Issue
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • parity
  • fether
  • Issues
  • #124
Closed
Open
Created Jul 03, 2018 by Ghost User@ghostContributor4 of 10 tasks completed4/10 tasks

Code Review, volunteers needed

@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.

Assignee
Assign to
Time tracking