Skip to content

Sample structure for ERC20 Data Facet - #397

Open
Jayy4rl wants to merge 12 commits into
Perfect-Abstractions:mainfrom
Jayy4rl:documentation-files
Open

Jayy4rl wants to merge 12 commits into
Perfect-Abstractions:mainfrom
Jayy4rl:documentation-files

Conversation

@Jayy4rl

@Jayy4rl Jayy4rl commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Created index and pages for ERC20 using the structure of the diamond facet documentation.

Changes Made

  • Created new folder "tokens"
  • Created an index for ERC20 subfolder
  • Full page documentation for ERC20 data facet
  • Skeleton for ERC20 Approve and Transfer

Tracking issue #369

@netlify

netlify Bot commented Sep 7, 2026

Copy link
Copy Markdown

Deploy Preview for compose-diamonds ready!

Name Link
🔨 Latest commit 8fdf3dc
🔍 Latest deploy log https://app.netlify.com/projects/compose-diamonds/deploys/6aaab481cf0f260008e1b6b6
😎 Deploy Preview https://deploy-preview-397--compose-diamonds.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@changeset-bot

changeset-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8fdf3dc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@maxnorm

maxnorm commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Hey @Jayy4rl, thanks for the PR. I've read the content from the changes

Could you please fix the build so we can see it in preview. It's failling due to missing link, you can just comment them out for now.

@Jayy4rl

Jayy4rl commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@maxnorm I've done that, should be able to preview now

Comment thread website/docs/library/token/ERC20/Data/ERC20DataFacet.mdx Outdated
@maxnorm

maxnorm commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Hey @Jayy4rl, this looks great for the Data facet. I like it.

How do you want to proceed next? You can build the whole ERC20 doc (facets/mods) with this PR or we merge this one and we do incremental PRs with the other pages. Whatever work best for you!

Thanks again for your help in writing documentation content. It's an important step for the adoption of Compose that most don't like to do.

Btw, feel free to propose improvements or fix things on the website as you work with it. (Maybe in separate PRs) I will be pleased to see your ideas.

@Jayy4rl

Jayy4rl commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Awesome!
That works. I'll push the rest of the ERC20 docs with this PR. Also will update if any proposals

@Jayy4rl

Jayy4rl commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Hey @maxnorm, For the index of the ERC20 folder, I found that listing the cards for all files was crowding the page. So I listed cards for the subfolders instead, created index.mdx files in each of the subfolders and then put the file cards in the respective subfolder index files. Its one extra click in navigation but i think it's neater. Can check the layout in the commit here 5d8e385

Also think i want to do the same thing for the Owners section. Diamond core section is fine as its only 4 cards. So let me know what you think.

@Jayy4rl

Jayy4rl commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

For the Permit mod, line 116 has this

* @dev Emits Approval event; must be emitted by the calling facet/contract.

But in the function actually emits the Approval event here

emit Approval(_owner, _spender, _value);

In the docs, I noted that the function emits the Approval event, so could you confirm that this is accurate? 04ec989

@maxnorm

maxnorm commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

For the Permit mod, line 116 has this

* @dev Emits Approval event; must be emitted by the calling facet/contract.

But in the function actually emits the Approval event here

emit Approval(_owner, _spender, _value);

In the docs, I noted that the function emits the Approval event, so could you confirm that this is accurate? 04ec989

We can remove the "must be emitted by the calling facet/contract" part in the function doc.

@Jayy4rl

Jayy4rl commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@maxnorm I've done that. Also found a small bug in the bridgeable mod and facet. I raised a PR here #400. Could you review that so i can complete the documentation for that section?

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Coverage

Metric Coverage Details
Lines 84% 3024/3602 lines
Functions 91% 721/789 functions
Branches 79% 352/446 branches

Last updated: Thu, 17 Sep 2026 19:06:30 GMT for commit 8fdf3dc

@github-actions

Copy link
Copy Markdown
Contributor

Gas Report

No gas usage changes detected between main and documentation-files.

All functions maintain the same gas costs. ✅

Last updated: Thu, 17 Sep 2026 19:06:51 GMT for commit 8fdf3dc

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.

4 participants