search

ubiquity / ubiquity-dollar

Ubiquity Dollar (UUSD) smart contracts and user interface.
https://uad.ubq.fi
Apache License 2.0
34 stars 89 forks source link

chore: Integrate solhint with a few rules to enforce contract convention #851

Closed molecula451 closed 9 months ago

molecula451 commented 9 months ago

Resolves #

Integrates solhint

https://github.com/ubiquity/ubiquity-dollar/assets/41552663/3836a9c5-088a-4882-8244-79e3537ee247

molecula451 commented 9 months ago

thoughts on having .solhint as one of the chosen linters for solidity? @rndquu @pavlovcik @gitcoindev

molecula451 commented 9 months ago

i think if we agree we can have the package already included, and selectively build these rules

0x4007 commented 9 months ago

thoughts on having .solhint as one of the chosen linters for solidity? @rndquu @pavlovcik @gitcoindev

I did limited research on the matter but 1. I recognize the package name as being pretty popular and 2. If it works then my vote is let's use it

Usually I review GitHub notifications from my phone. For some reason on both my browser and the GitHub app, videos don't load so I don't have all the context.

ubiquibot[bot] commented 9 months ago 
! action returned an unexpected value
molecula451 commented 9 months ago

thoughts on having .solhint as one of the chosen linters for solidity? @rndquu @pavlovcik @gitcoindev

I did limited research on the matter but 1. I recognize the package name as being pretty popular and 2. If it works then my vote is let's use it

Usually I review GitHub notifications from my phone. For some reason on both my browser and the GitHub app, videos don't load so I don't have all the context.

ahhh.. the well known video Github app issue, out of context: (i wonder why they don't fix it, it still happens, lol)

dropboxed to you @pavlovcik : https://www.dropbox.com/scl/fi/qcmrhzg04zrqvdw97syni/solhint.mp4?rlkey=cvli23m2am7x0ekxidf6um0qk&dl=0

0x4007 commented 9 months ago

thoughts on having .solhint as one of the chosen linters for solidity? @rndquu @pavlovcik @gitcoindev

I did limited research on the matter but 1. I recognize the package name as being pretty popular and 2. If it works then my vote is let's use it Usually I review GitHub notifications from my phone. For some reason on both my browser and the GitHub app, videos don't load so I don't have all the context.

ahhh.. the well known video Github app issue, out of context: (i wonder why they don't fix it, it still happens, lol)

dropboxed to you @pavlovcik : https://www.dropbox.com/scl/fi/qcmrhzg04zrqvdw97syni/solhint.mp4?rlkey=cvli23m2am7x0ekxidf6um0qk&dl=0

GitHub iOS ironically is not open sourced. I tried researching and implementing a fix already 👀

rndquu commented 9 months ago

I'm ok to use solhint. It's security rules are pretty outdated (slither works much better) but style guide and best practices rules could be useful.

molecula451 commented 9 months ago

im mergin but not closing the issue #849 since this is only the ingration of the module, so the issue serves to craft further rules that'll we be using

ubiquibot[bot] commented 9 months ago 
# Comment event received without a recognized user command.
ubiquibot[bot] commented 9 months ago 
@@ No latest assign event found. @@
rndquu commented 9 months ago

@molecula451 Could you add .solhint.json to exceptions here to make "Enforce kebab-case" workflow pass without errors?

molecula451 commented 9 months ago

@molecula451 Could you add .solhint.json to exceptions here to make "Enforce kebab-case" workflow pass without errors?

can you back me up on this one? if not i will PR in a bit

gitcoindev commented 9 months ago

@molecula451 Could you add .solhint.json to exceptions here to make "Enforce kebab-case" workflow pass without errors?

can you back me up on this one? if not i will PR in a bit

I opened https://github.com/ubiquity/ubiquity-dollar/pull/853