eth-sri / securify

[DEPRECATED] Security Scanner for Ethereum Smart Contracts
Apache License 2.0
215 stars 50 forks source link

add pattern descriptions to output #59

Closed hiqua closed 5 years ago

hiqua commented 5 years ago

I'm not sure we want to add this to the output directly, given that it will clutter it more than necessary.

Closes #12.

ghost commented 5 years ago

DeepCode analyzed this pull request. There are no new issues.

ritzdorf commented 5 years ago

I guess it could be added to the output in case a --verbose flag is used. Or what were you thinking @hiqua ?

hiqua commented 5 years ago

In that case I would add the information to the pattern classes directly, and remove the files I've added in the previous commit, so that we have all the information only once in the repo (and we don't parse the description json to export it again). Fine for you?

ghost commented 5 years ago

DeepCode analyzed this pull request. There are no new issues.

hiqua commented 5 years ago

Done, see the new flag, and the output when you enable it.

Note that 05632895d6b0dc15c9ec284bb877020ce225f5cc is not really related to this PR, but it's small, and some of the tests that had to be updated for this PR wouldn't have required update if this commit had taken place earlier.

hiqua commented 5 years ago

Can we omit the catIdToName part from the output?

Done.

hiqua commented 5 years ago

I know that they are in the --help but sometimes it can still be helpful to add them to the README from my perspective.

I agree that we should list the main features in the README, however I don't think we should try and have the exhaustive list of options there, because it is bound to become outdated at one point, so it's better to refer to --help. I always prefer to have the information in one place, for many things, for this reason.

Either a potential user is looking into what's feasible, and then having a high-level description is better, either the user already has Securify running, in which case it's simpler to run with --help.