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

Chore: add github actions #30

Closed
wants to merge 3 commits into from
Closed

Conversation

haythemsellami
Copy link
Member

@haythemsellami haythemsellami commented Mar 14, 2024

This PR:

  • Adds github actions script
    • It will fail on every non formatted new code
  • Format existent code using forge fmt
  • Add remappings.txt file

@haythemsellami haythemsellami self-assigned this Mar 14, 2024
@haythemsellami haythemsellami changed the title [WIP]Chore: add github actions Chore: add github actions Mar 14, 2024
foundry.toml Outdated
Comment on lines 28 to 33
ignore = [
"src/EVault/EVault.sol",
"src/EVault/shared/Storage.sol",
"src/EVault/shared/types/MarketStorage.sol",
"src/EVault/shared/types/MarketCache.sol"
]
Copy link
Member Author

@haythemsellami haythemsellami Mar 14, 2024

Choose a reason for hiding this comment

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

I removed this.
Is there any specific reason those were excluded from formatting?

Copy link
Contributor

Choose a reason for hiding this comment

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

@dglowinski any reason for it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, more readability ;) like storage slots separated by blank spaces

Evault especially we want to have one line per function

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why put remappings in a file? We don't do it in EVC

Copy link
Member Author

Choose a reason for hiding this comment

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

I think from readability side, it is easier to read when formatted than having a really long line in EVault XD

Copy link
Collaborator

Choose a reason for hiding this comment

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

only if you're after reading the whole line, which is probably not why you go to that file. It's mostly boilerplate, what's interesting is the nice list of functions you can quickly scan. There's not much interesting happening in the lines to the right of the function name

Copy link
Member Author

@haythemsellami haythemsellami Mar 15, 2024

Choose a reason for hiding this comment

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

Will close this for now and open new one in a bit.

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.

3 participants