Skip to content

Remove the arg-overflow rule, with tests - #8

Open
come-maiz wants to merge 4 commits into
OpenZeppelin:masterfrom
come-maiz:tests
Open

Remove the arg-overflow rule, with tests#8
come-maiz wants to merge 4 commits into
OpenZeppelin:masterfrom
come-maiz:tests

Conversation

@come-maiz

Copy link
Copy Markdown

No description provided.

Comment thread test/solium/no-arg-overlow-rule.js Outdated
let dedent = require("dedent");
let Solium = require("solium");

let userConfig = JSON.parse(fs.readFileSync('.soliumrc.json', 'utf8'));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why let instead of const? Do we have (should we have) a rule for this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've proposed #9 to add this rule.

Now this brings a super good topic, we were not running the linter on this code. I've updated the branch to do so.
TODO for later: add tests for the eslint rules.


errors.should.deepEqual([]);

done();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I thought done() was only used when doing async tests without async/await?

Comment thread test/solium/no-arg-overlow-rule.js Outdated
function testFunc(uint arg1, uint arg2, uint arg3, uint arg4) public {
}
}`,
errors = Solium.lint(code, userConfig);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I find the let a = expr, b = expr syntax confusing, is there a reason to have both assignments on the same line?

@come-maiz

Copy link
Copy Markdown
Author

@nventuro the answer to all your questions is that I've just copied the style from solium. So I appreciate a lot your comments, they help me find what's good JS and what's not.

Without calling done it fails. This are the docs I read about it:
https://mochajs.org/#asynchronous-code
I'm not sure if you are proposing an alternate approach. I've left it there for now.

I've pushed an update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants