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

Docs: Add introduction documentation for cairo vm - Issue #328 #381

Merged
merged 8 commits into from
Jun 12, 2024

Conversation

coxmars
Copy link
Contributor

@coxmars coxmars commented Apr 30, 2024

This PR is for the issue Intro Documentation #328. Added intro documentation to understand in a general way all the sections as a small summary of what we'll see in the specific parts.

Copy link
Contributor

@TAdev0 TAdev0 left a comment

Choose a reason for hiding this comment

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

Hey @coxmars

I left a first bunch of comments on your PR but it looks good overall!!

Did you use AI for the part about builtins? We should avoid things like:

" These include managing resource usage effectively to prevent performance degradation or vulnerabilities, ensuring robust security measures to mitigate risks such as integer overflow, reentrancy attacks, and access control issues, and maintaining compatibility with existing standards and protocols within the blockchain ecosystem. "

docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
@coxmars
Copy link
Contributor Author

coxmars commented May 2, 2024

Hey @coxmars

I left a first bunch of comments on your PR but it looks good overall!!

Did you use AI for the part about builtins? We should avoid things like:

" These include managing resource usage effectively to prevent performance degradation or vulnerabilities, ensuring robust security measures to mitigate risks such as integer overflow, reentrancy attacks, and access control issues, and maintaining compatibility with existing standards and protocols within the blockchain ecosystem. "

Thank you for the feedback, I will apply the changes as soon as possible and I will modify that sentence to avoid that kind of explanation

@coxmars
Copy link
Contributor Author

coxmars commented May 3, 2024

Hi @TAdev0 can you review these changes please, I considered all your comments for this improvement but if something is needed to improve I will do it, thanks.

Copy link
Contributor

@TAdev0 TAdev0 left a comment

Choose a reason for hiding this comment

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

Left another bunch of comments! @coxmars

docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
@cicr99 cicr99 linked an issue May 6, 2024 that may be closed by this pull request
@TAdev0
Copy link
Contributor

TAdev0 commented May 7, 2024

hi @coxmars any update on this?

@coxmars
Copy link
Contributor Author

coxmars commented May 8, 2024

hi @coxmars any update on this?

I'm finishing applying the changes, I'll let you know as soon as I have it.

@rodrigo-pino
Copy link
Contributor

Hey @coxmars if you need any help please let us know

@coxmars
Copy link
Contributor Author

coxmars commented May 14, 2024

Hey @coxmars if you need any help please let us know

Hi Rodrigo, I would like you to give me some context/guidance on the resources section, what do you mean by that? I did not find related information or maybe I did not understand the specific approach to address the explanation. Thank you.

docs/package.json Outdated Show resolved Hide resolved
docs/package-lock.json Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
docs/docs/vm-fundamentals/intro.md Outdated Show resolved Hide resolved
@coxmars
Copy link
Contributor Author

coxmars commented May 25, 2024

@TAdev0 improvements applied if you need other changes just let me know, thanks

docs/docs/vm-fundamentals/intro.md Show resolved Hide resolved
@cicr99 cicr99 merged commit b3d6352 into NethermindEth:main Jun 12, 2024
4 checks passed
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.

Documentation Intro
4 participants