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

update general/conflux-basics/gas.md, regarding the issue #451 #453

Merged
merged 4 commits into from
Mar 14, 2024

Conversation

jackleeio
Copy link
Contributor

@jackleeio jackleeio commented Mar 13, 2024

Pre-flight checklist


This change is Reviewable

Copy link

vercel bot commented Mar 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
conflux-documentation ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 14, 2024 8:22am

@jackleeio jackleeio changed the title update general/conflux-basics/gas.md, Regarding the issue #451 update general/conflux-basics/gas.md, regarding the issue #451 Mar 13, 2024
Copy link
Collaborator

@darwintree darwintree left a comment

Choose a reason for hiding this comment

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

Thank you for your pull request. There are some changes needed before this pull request could get merged.

  1. The position where the content is inserted. Currently the new part is inserted into between How gasFee is calculated and the subsection gasUsed. It is supposed to move somewhere else to retain context consistency
    image
  2. the second point and third point are referring the same thing ---- sponsorship mechanism. And I don't think this is the suitable contents here. From the user's perspective, he has hardly choice making use of the sponsorship mechanism or not. And from developers' perspective, the developer did not pay less.
  3. The header "How do I pay less gas?" should be changed. For example, "How can users pay less gas fee?". And relating contents can be illustrated from user's or developer's perspective, for example, optimizing contract logic, using multicall, etc

@jackleeio
Copy link
Contributor Author

Made some changes as per your tips

@darwintree
Copy link
Collaborator

Made some changes as per your tips

The modified content is good! And for the current content, I think it would be better to be listed as an individual article because the pages in general/conflux-basics folder will mainly focus on general concepts rather than detailed development guide. You can move the gas optimization suggestion to create a new page in general/build/contracts as you suggested in #454. Then you can link the new page in this page.

Another question is the a series of tutorials for gas optimization. I guess you are going to create detailed tutorials, for example, illustrate the mechanism and giving an example on how to use libraries. In that case that would be awesome. But it is also a choice that you only contain the most important key points in a single article. But anyway it depends on you.

@jackleeio
Copy link
Contributor Author

I made some more modifications according to your suggestions and made some improvements for Issue #454. After that, I will gradually add detailed tutorials on gas optimization. If time permits, I will use my free time to add a tutorial every day.

@darwintree
Copy link
Collaborator

I think after the above minor change, the current content is ready to be merged. Would you like to be merged immediately or after all the relating gas optimization tutorials are ready?

Copy link
Collaborator

@darwintree darwintree left a comment

Choose a reason for hiding this comment

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

lgtm

@jackleeio
Copy link
Contributor Author

thanks, bro

@darwintree darwintree merged commit 0698cd5 into Conflux-Chain:main Mar 14, 2024
1 check 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.

2 participants