-
Notifications
You must be signed in to change notification settings - Fork 0
PR #1
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
base: initial
Are you sure you want to change the base?
Conversation
ppoliani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Replace all
letwithconst - Avoid using
function; preferconstinstead
app.js
Outdated
| run(); | ||
|
|
||
| //main function | ||
| async function run(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async 👍
app.js
Outdated
| run(); | ||
|
|
||
| //main function | ||
| async function run(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one suggestion; since we're using the functional programming paradigm we really like to follow the principles from the style of programming.
one of the suggestion is to use name binding. This applies for functions, as well.
This can be rewritten like this:
const run = async () => {}
app.js
Outdated
|
|
||
| //main function | ||
| async function run(){ | ||
| let b_addresses = Object.keys(balancesheet); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer const over let. It's nice to keep reference transparency
app.js
Outdated
| //main function | ||
| async function run(){ | ||
| let b_addresses = Object.keys(balancesheet); | ||
| let b_values = Object.values(balancesheet); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here const
app.js
Outdated
| let TokenDistribution = await deployContract('./build/contracts/TokenDistribution.json', account, [CappedMintableToken.options.address]); | ||
|
|
||
|
|
||
| await CappedMintableToken.methods.mint(TokenDistribution.options.address, 15000).send({from: account.address}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
await CappedMintableToken.methods.mint(TokenDistribution.options.address, 15000, {from: account.address})
app.js
Outdated
| }; | ||
|
|
||
| //helper function to deloy contracts | ||
| function deployContract(contractPath, account, args){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const deployContract = (contractPath, account, args) => {}
contracts/CappedMintableToken.sol
Outdated
| @@ -0,0 +1,12 @@ | |||
| pragma solidity ^0.5.0; | |||
|
|
|||
| import "../node_modules/openzeppelin-solidity/contracts/token/ERC20/ERC20Capped.sol"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import "openzeppelin-solidity/token/ERC20/ERC20Capped.sol";
contracts/CappedMintableToken.sol
Outdated
|
|
||
| constructor(uint256 cap) ERC20Capped(cap) public {} | ||
|
|
||
| function mint(address _to, uint256 _amount) public returns (bool){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can remove this function because it will be inherited from the ERC20Capped
contracts/TokenDistribution.sol
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace missing
app.js
Outdated
| function deployContract(contractPath, account, args){ | ||
| return new Promise((resolve,reject)=>{ | ||
| // Read the JSON file contents | ||
| let contractJsonContent = fs.readFileSync(contractPath, 'utf8'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use promisify https://nodejs.org/dist/latest-v8.x/docs/api/util.html and then use async/await
No description provided.