Skip to content

refactor done seprated constants from config #839

New issue

Have a question about this project? Sign up for a free account to open an issue and contact its maintainers and the community.

By clicking “Sign up for ”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on ? Sign in to your account

Merged
merged 8 commits into from
Aug 5, 2022

Conversation

LijuJoseJJ
Copy link
Contributor

Copy link
Contributor

@Westlad Westlad left a comment

Choose a reason for hiding this comment

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

I think we need to be careful about the definition of a config item and a constant. A constant will never depend on the details of our deployment. An example is ZERO: 0x000..... A config item is something that is deployment dependent, for example the name of a Mongo collection or the height of a Merkle tree. There are only a few grey areas, such as the definition of calldata types. These could arguably be either but are best categorised as constants because they will actually never be dependent on the details of a deployment.

@LijuJoseJJLijuJoseJJ force-pushed the liju.jose/refactor-config-and-consts branch from c988034 to adae5b3 Compare August 2, 2022 13:04
@LijuJoseJJLijuJoseJJ requested a review from Westlad August 3, 2022 04:55
@LijuJoseJJ
Copy link
Contributor Author

I think we need to be careful about the definition of a config item and a constant. A constant will never depend on the details of our deployment. An example is ZERO: 0x000..... A config item is something that is deployment dependent, for example the name of a Mongo collection or the height of a Merkle tree. There are only a few grey areas, such as the definition of calldata types. These could arguably be either but are best categorised as constants because they will actually never be dependent on the details of a deployment.

changes accommodated

@WestladWestlad requested a review from druiz0992 August 4, 2022 10:47
@LijuJoseJJLijuJoseJJ added the One more approval neededOne reviewer has approved this PR but another is neededlabel Aug 5, 2022
@druiz0992druiz0992 merged commit 6220421 into master Aug 5, 2022
@druiz0992druiz0992 deleted the liju.jose/refactor-config-and-consts branch August 5, 2022 06:24
Sign up for free to join this conversation on . Already have an account? Sign in to comment
Labels
Changes to deployment One more approval neededOne reviewer has approved this PR but another is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants