Skip to content
This repository has been archived by the owner on Mar 11, 2024. It is now read-only.

Lint Migrations.sol #3

Merged
merged 1 commit into from
Feb 12, 2018
Merged

Lint Migrations.sol #3

merged 1 commit into from
Feb 12, 2018

Conversation

cgewecke
Copy link
Contributor

@cgewecke cgewecke commented Jan 18, 2018

Addresses truffle-migrate 19

  • Compiles (Remix w/ solc 0.4.19).
  • Lints (used Solhint v 1.1.9, as suggested by the slack thread)
  • Solhint warns about not locking the pragma. However a best-practices issue here arrived at the conclusion that libraries and platforms should not pin.

  • Adds public modifier to everything including constructor. More on constructor visibility over at Solidity here

Remix warns that:

Gas requirement of function Migrations.upgrade(address) high: infinite. If the gas requirement of a function is higher than the block gas limit, it cannot be executed. Please avoid loops in your functions or actions that modify large areas of storage (this includes clearing or copying arrays in storage)

Probably fine for most people though :)


function upgrade(address new_address) restricted {
Migrations upgraded = Migrations(new_address);
upgraded.setCompleted(last_completed_migration);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does solhint want 4-space indents?

Copy link
Contributor Author

@cgewecke cgewecke Feb 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my worldview!!

@cgewecke cgewecke merged commit eeda015 into develop Feb 12, 2018
@cgewecke cgewecke deleted the lintable-migrations branch February 12, 2018 18:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants