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

Makes comments and formatting consistent #235

Merged
merged 8 commits into from
Jun 25, 2023

Conversation

gigamesh
Copy link
Contributor

@gigamesh gigamesh commented Mar 4, 2023

Please don't be alarmed by the diff. None of the changes will change the bytecode.

Given the significance of EIP-4337 and how much use these contracts will receive, I thought it would be beneficial to make the comments more consistently natspec compliant and make the line lengths & indentation consistent for better readability. I tried not to be too opinionated and followed established patterns (used same @param description indentation pattern as Seaport).

Summary:

  • Limits line character count
  • Indentation consistency
  • Punctuation consistency
  • Consistent natspec formatting
    • Adds missing @param descriptions
    • Uses @inheritdoc for the functions that can use it but weren't already
  • Makes multiline comments use /***/

I'm happy to make any changes. And if you prefer to not merge this, no problem.

Note: I didn't touch any of the contracts in /samples and /test but happy to do those as well. Also didn't add descriptions for events, errors, constructor and receive functions that are missing them.

@gigamesh gigamesh force-pushed the gigamesh/comments branch from e93160f to ebbf21a Compare March 4, 2023 22:14
@gigamesh gigamesh force-pushed the gigamesh/comments branch from adef638 to 64cbdf6 Compare March 4, 2023 22:33
@gigamesh
Copy link
Contributor Author

Alright well I guess this is just going to get ignored. 😞

@gigamesh gigamesh closed this Apr 25, 2023
@shahafn shahafn reopened this May 4, 2023
@shahafn
Copy link
Contributor

shahafn commented May 4, 2023

Hi @gigamesh sorry for not responding earlier. We'll review the PR. Thank you for the effort!

@drortirosh
Copy link
Contributor

Thank you.
we will merge it, but only before next release, as we want to keep the contracts compatible with the on-chain contracts, and any source change (even comment) generates slight different bytecode..

@drortirosh drortirosh added the defer-to-next-version Should be merged when we have some other breaking change. label Jun 12, 2023
@gigamesh
Copy link
Contributor Author

Thank you. we will merge it, but only before next release, as we want to keep the contracts compatible with the on-chain contracts, and any source change (even comment) generates slight different bytecode..

Much appreciated!

@forshtat forshtat changed the base branch from develop to next_v0.7 June 25, 2023 18:38
@forshtat forshtat merged commit 10f0502 into eth-infinitism:next_v0.7 Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defer-to-next-version Should be merged when we have some other breaking change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants