Skip to content

update ink! attribute argument syntax docs and simplify ink! attribute meta parsing implementation in ink_ir#1927

Closed
davidsemakula wants to merge 2 commits intouse-ink:masterfrom
davidsemakula:ast
Closed

update ink! attribute argument syntax docs and simplify ink! attribute meta parsing implementation in ink_ir#1927
davidsemakula wants to merge 2 commits intouse-ink:masterfrom
davidsemakula:ast

Conversation

@davidsemakula
Copy link
Copy Markdown
Collaborator

@davidsemakula davidsemakula commented Sep 28, 2023

Summary

Closes #_

  • [n] y/n | Does it introduce breaking changes?
  • [n] y/n | Is it dependant on the specific version of cargo-contract or pallet-contracts?

Description

  • Updates ink! attribute argument syntax documentation in ink_ir's ast module to accurately describe current deviations from Rust "meta item" attribute syntax.
  • Simplifies ink! attribute meta parsing implementation in ink_ir's ast::meta module by removing unnecessary custom utilities while maintaining support for parsing the described ink! attribute argument syntax.

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have added an entry to CHANGELOG.md
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@davidsemakula davidsemakula changed the title update rustdoc for ink_ir ast module and simplify ink! attribute meta parsing implementation and update ink! attribute argument syntax docs and simplify ink! attribute meta parsing implementation in ink_ir Sep 28, 2023
@cmichi
Copy link
Copy Markdown
Collaborator

cmichi commented Mar 8, 2024

Sorry for not providing feedback earlier, @davidsemakula. Is that why you closed the PR?

@davidsemakula
Copy link
Copy Markdown
Collaborator Author

Hi @cmichi , no problem ... I just decided that it was better to include changes from this that are still relevant in more recent/new PRs 🙂

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