Add linting for vyper files #52
Conversation
spinoch
force-pushed
the
add_vyper_linting
branch
from
241bc2e to
07bb6ff
Compare
|
Not sure why the brownie workflow is failing on the pre-commit hook check (I just reformatted the file). Will look into it at another time. |
spinoch
force-pushed
the
add_vyper_linting
branch
2 times, most recently
from
cdee4e9 to
9e66129
Compare
|
Of course, I had to rebase. I consider this ready for review now. |
There was a problem hiding this comment.
This is really awesome, thank you! I just have one modification I'd like to see if it's possible to change black to accept.
Also, in terms of distribution, will you be able to publish to PyPI so we can more officially integrate this into our CI? (through adding another task)
contracts/Vault.vy
Outdated
| def name() -> String[42]: view | ||
| def symbol() -> String[20]: view | ||
| def decimals() -> uint256: view |
There was a problem hiding this comment.
Can blackadder be adjusted to maintain this? I find this much more readable actually (the item after the : is actually the mutability of the method, not a function body). We don't use classes for anything else, so perhaps this behavior can be monkey-patched
There was a problem hiding this comment.
I guess we'd have to patch black's transform_line or the Line class. It's doable, but will require some investigation/time
I find this much more readable
agree. And aesthetic
There was a problem hiding this comment.
fyi I probably won't be able to look into this until next week
There was a problem hiding this comment.
What's weird about this is that Strategy interface below didn't get updated... 🤔
There was a problem hiding this comment.
Not sure I get what you mean, I see Strategy reformatted below here
There was a problem hiding this comment.
Weird, I don't
interface Strategy:
def distributeRewards(_shares: uint256): nonpayable
def estimatedTotalAssets() -> uint256: view
def withdraw(_amount: uint256): nonpayable
def migrate(_newStrategy: address): nonpayable
There was a problem hiding this comment.
ah, sorry! that's why that CI keeps failing haha
There was a problem hiding this comment.
@fubuloubu for now, I'd suggest to keep this formatting as is, if it's not too much of an inconvenience. We should address these formatting issues in the next major blackadder release but it will take some time until we get that out.
Otherwise, we can postpone this PR until then.
There was a problem hiding this comment.
I'd argue we should wait then. This reformatting makes things significantly less clear.
Sure, I should add a step to the blackadder CI so that it automatically pushes on new tags. |
|
This is now on PyPI, https://pypi.org/project/blackadder/
Shall I replace the |
Yes please! Also if you could make it another task in the Lint config, that'd be awesome! |
spinoch
force-pushed
the
add_vyper_linting
branch
from
f12c1fd to
e5f66e8
Compare
spinoch
force-pushed
the
add_vyper_linting
branch
from
e5f66e8 to
93c1fb9
Compare
|
Will have to re-approach after this tool is ready for primetime. Thanks for giving it a shot! |
Fixes #16