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: bump solhint, integrate Husky and lint-staged #1026

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

smol-ninja
Copy link
Member

Changelog

Note

To bypass husky hook, just add -n to your commit (from git docs).

> git commit -S -n -m "your message"

Copy link
Member

@PaulRBerg PaulRBerg left a comment

Choose a reason for hiding this comment

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

very cool, thanks for this.

I wish this existed when we wrote the first lines of code in 2022!

@andreivladbrg
Copy link
Member

andreivladbrg commented Aug 27, 2024

Before giving a full review, I want to mention that we should rebase in staging from main as, for example, the formatting in precompiles was already addressed

I will rebase - hope not to cause conflicts in the PRs

@andreivladbrg
Copy link
Member

Coming back re rebase, since there are conflicts between main and staging and many PRs open to staging, I will postpone until we merge all of them into staging (expect for this one) and then rebase from main.

@smol-ninja
Copy link
Member Author

smol-ninja commented Aug 27, 2024

Even if formatting in precompiles was already addressed, why cant we merge it in staging and rebase later (in the end)? Resolving formatting conflicts isn't that hard.

@andreivladbrg
Copy link
Member

why cant we merge it in staging and rebase later (in the end)?

we can, but the more commits there are in the staging branch, the more conflicts you’ll need to resolve.

Resolving formatting conflicts isn't that hard

well, it’s not difficult in terms of complexity, but the more conflicts you have to resolve, the more likely you are to miss something
for example, this issue: sablier-labs/v2-periphery#373 shouldn't have not existed, as the struct was added by mistake when rebased

@smol-ninja smol-ninja force-pushed the bump-solhint branch 2 times, most recently from 0bcf94e to 354de18 Compare August 28, 2024 16:30
Copy link
Member

@andreivladbrg andreivladbrg left a comment

Choose a reason for hiding this comment

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

The PR looks good, since foundry reverted the formatting style, let's merge this 👍

@smol-ninja
Copy link
Member Author

Great. Merging now.

@smol-ninja smol-ninja merged commit 2534f59 into staging Sep 9, 2024
9 checks passed
@smol-ninja smol-ninja deleted the bump-solhint branch September 9, 2024 15:23
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