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

Use a single command to do prepare for PR #19

Closed
3esmit opened this issue Feb 24, 2024 · 4 comments · Fixed by #23
Closed

Use a single command to do prepare for PR #19

3esmit opened this issue Feb 24, 2024 · 4 comments · Fixed by #23
Assignees
Labels
type: feature New feature or request

Comments

@3esmit
Copy link

3esmit commented Feb 24, 2024

The PR request that we do several steps...

I recommend that some of these steps are done together in a single command:

The problem I am facing is that I always forget to run one of these commands, causing me to have to force update the branch to avoid a single commit just for "lint", "snapshot" or "gas-report".

I would recommend that we run pnpm prepare that is pnpm prettier:write && forge fmt && forge snapshot && pnpm gas-report and ask in checklist to run that instead of these separated commands.

@0x-r4bbit
Copy link
Collaborator

Yea, I think generally this wouldn't hurt.
Just keep in mind that, even if we hide these sub commands behind a single umbrella command, this won't change the fact that you might have to amend your commit(s) and force update your branch afterwards.

Also, I think the prepare command should also do pnpm prettier:write to account for non solidity files.

@0x-r4bbit 0x-r4bbit added the type: feature New feature or request label Feb 26, 2024
@3esmit
Copy link
Author

3esmit commented Feb 26, 2024

Can we run these commands with a pre-commit hook?
See https://prettier.io/docs/en/precommit

@0x-r4bbit
Copy link
Collaborator

Yes, we can set one up I believe.
I found pre-commit hooks a bit annoying in practice, but happy to give this a try if this improves everyone's workflows!

@3esmit
Copy link
Author

3esmit commented Feb 26, 2024

Maybe for the pre-commit just run the forge fmt and prettier write, and for the files that were changed. The verify, test, report might be a little bit too overhead for big projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

2 participants