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

Add scripts to check Court bytecode size #119

Closed
wants to merge 1 commit into from

Conversation

bingen
Copy link
Contributor

@bingen bingen commented Sep 5, 2019

No description provided.

@bingen bingen self-assigned this Sep 5, 2019
Copy link
Contributor

@facuspagnuolo facuspagnuolo left a comment

Choose a reason for hiding this comment

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

Not sure what's the intent of this CI task, the idea is simply to have the CI failing in case the Court contract size hits the limit?

@bingen
Copy link
Contributor Author

bingen commented Sep 6, 2019

Yes. I know that tests should fail, but at some point I was not confident about it, although I'm not sure anymore, as I guess that when that happened to me in the past was probably because I was measuring it wrong.
Although I think it does not hurt, we can remove the CI task if you are completely sure we can rely on regular tests and leave only the scripts, which are very useful to check the evolution and how we are doing, as we are often on the bleeding edge.

@facuspagnuolo
Copy link
Contributor

I think we can trust our test-suit, EIP-170 is implemented in ganache, and even more, once #87 is implemented, we will be more sure about it. I agree it could be useful to be aware of how far we are of the size limit, but in that case, we should turn those scripts into a github hook or similar, otherwise, I don't think we will remember to run it locally every time we make a change to contracts

@bingen
Copy link
Contributor Author

bingen commented Sep 6, 2019

we should turn those scripts into a github hook or similar,

Good idea, I'll do that!

@facuspagnuolo
Copy link
Contributor

Can we close this one for now?

@facuspagnuolo
Copy link
Contributor

Closing this one for now

@izqui izqui added this to the Freeze #1 milestone Oct 15, 2019
@facuspagnuolo facuspagnuolo deleted the bytecode_size_helpers branch October 17, 2019 14:25
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.

3 participants