Skip to content
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

Validate storage keys' length and improve doc #565

Merged
merged 3 commits into from
Jul 26, 2019

Conversation

alcuadrado
Copy link
Member

Closes #557

@lgtm-com
Copy link

lgtm-com bot commented Jul 25, 2019

This pull request introduces 1 alert when merging 95ec40a into 4bbb6e3 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@coveralls
Copy link

coveralls commented Jul 25, 2019

Coverage Status

Coverage increased (+0.02%) to 95.114% when pulling 91e2d43 on state-manager-buffer-lengths into 59743f2 on master.

@holgerd77
Copy link
Member

Just a note on first look: "Remove LGTM warning" is not an optimal commit message since this hides the change itself being done and is not really readable in the commit message.

@holgerd77
Copy link
Member

(sorry, a bit nit-picking, but "Remove unused import in StateManager" or something would just be more expressive).

@holgerd77
Copy link
Member

@s1na I think these changes/improvements make a lot of sense and would have a tendency to approve and merge (after another PRs CI has been run through and I can update this branch). If you have any objects in between while reading let me know.

@s1na
Copy link
Contributor

s1na commented Jul 26, 2019

Looks good to me

@holgerd77 holgerd77 merged commit aa0bf49 into master Jul 26, 2019
@holgerd77 holgerd77 deleted the state-manager-buffer-lengths branch July 26, 2019 11:21
@alcuadrado
Copy link
Member Author

Just a note on first look: "Remove LGTM warning" is not an optimal commit message since this hides the change itself being done and is not really readable in the commit message.

That's a good point @holgerd77! I'll write more descriptive commit messages in the future :)

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

Successfully merging this pull request may close these issues.

StateManager#getContractStorage's buffer sizes
5 participants