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

[FIX] Improve writing problems + fix several typos #332

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

azarpoor
Copy link

No description provided.

@azarpoor
Copy link
Author

@kdeme @ogenev

#### Contract Code

These data types represent the bytecode for a contract.

> NOTE: Because CREATE2 opcode allows for redeployment of new code at an existing address_hash, we MUST randomly distribute contract code storage across the DHT keyspace to avoid hotspots developing in the network for any contract that has had many different code deployments. Were we to use the path based *high-bits* approach for computing the content-id, it would be possible for a single location in the network to accumulate a large number of contract code objects that all live in roughly the same space.
Problematic!
> NOTE: Because CREATE2 opcode allows for redeployment of new code at an existing address*hash, we MUST randomly distribute contract code storage across the DHT keyspace to avoid hotspots developing in the network for any contract that has had many different code deployments. Were we to use the path based \_high-bits* approach for computing the content-id, it would be possible for a single location in the network to accumulate a large number of contract code objects that all live in roughly the same space.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like a faulty change.

Copy link
Author

Choose a reason for hiding this comment

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

hey again yes you right my bad

@azarpoor azarpoor requested a review from kdeme September 2, 2024 14:16
@azarpoor
Copy link
Author

azarpoor commented Sep 6, 2024

@kdeme

Copy link
Collaborator

@kdeme kdeme left a comment

Choose a reason for hiding this comment

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

Great, thanks for fixing typos, removing trailing whitespaces, fixing notes.

But regarding list styles, indents, emphasis markers and so on (anything that does not break markdown specs), that seems like personal preference changes that are also difficult to maintain without having a linter applied in CI.

So in my opinion, these type of changes should get a linter added first for some consistent settings, else somebody else tomorrow might just set them all back to their liking.

@azarpoor
Copy link
Author

azarpoor commented Sep 9, 2024

Great, thanks for fixing typos, removing trailing whitespaces, fixing notes.

But regarding list styles, indents, emphasis markers and so on (anything that does not break markdown specs), that seems like personal preference changes that are also difficult to maintain without having a linter applied in CI.

So in my opinion, these type of changes should get a linter added first for some consistent settings, else somebody else tomorrow might just set them all back to their liking.

correct 👍

do we have a general linter for repo? if we dont have a proper linter for repo we can implement a structure for this one

@kdeme
Copy link
Collaborator

kdeme commented Sep 10, 2024

do we have a general linter for repo? if we dont have a proper linter for repo we can implement a structure for this one

No, we don't. I've created an issue for this some time back: #321

My suggestion would be to adjust this PR to only contain actual fixes.

Any additional PR that then introduces linting is highly welcomed of course :)

@azarpoor
Copy link
Author

azarpoor commented Sep 10, 2024

do we have a general linter for repo? if we dont have a proper linter for repo we can implement a structure for this one

No, we don't. I've created an issue for this some time back: #321

My suggestion would be to adjust this PR to only contain actual fixes.

Any additional PR that then introduces linting is highly welcomed of course :)

nice !

yeah of course , i will change necessary changes on this pull req 👍

and yeah i enjoy giving structural improve :)) i try my best for making another pull req for linting purpose 👍

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