search

getappmap / scanner

Code scanning, linting, assertions and alerts.
Other
0 stars 0 forks source link

feat: each Rule defines whether it's enabled by default #89

Closed kgilpin closed 2 years ago

kgilpin commented 2 years ago

Remove the need for default.yml, by enabling each Rule to declare whether it's default-enabled.

Default-enabled rules can be disabled in the configuration:

disableDefault:
- http-500

This way, a scanner configuration file does not have to re-declare all the default rules.

dividedmind commented 2 years ago

I don't like this idea. The default configuration file serves as a central reference for the available configuration and a useful base for the user's configuration.

kgilpin commented 2 years ago

How are we going to enable new scanners? Once a user has decided to provide their own configuration, they will have to modify it to add any new rules that we come out with.

On Tue, Jan 25, 2022 at 9:44 AM Rafał Rzepecki @.***> wrote:

I don't like this idea. The default configuration file serves as a central reference for the available configuration and a useful base for the user's configuration.

— Reply to this email directly, view it on GitHub https://github.com/applandinc/scanner/pull/89#issuecomment-1021255221, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAVC627Z3V4M255JDAYHJLUX2ZL3ANCNFSM5MWDLHYQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

kgilpin commented 2 years ago

I will convert this to a Draft until we have a chance to discuss it.

dividedmind commented 2 years ago

How are we going to enable new scanners? Once a user has decided to provide their own configuration, they will have to modify it to add any new rules that we come out with.

Not necessarily. The final configuration would simply be {...defaultConfig, ...userConfig} (maybe globalConfig, too, eg. ~/.config/appland/scanner.yaml). This is how most tools work and I think it's pretty expected.

kgilpin commented 2 years ago

Ok so you’re fine with merging the configs you just want the default to be more explicit?

On Wed, Jan 26, 2022 at 12:32 PM Rafał Rzepecki @.***> wrote:

How are we going to enable new scanners? Once a user has decided to provide their own configuration, they will have to modify it to add any new rules that we come out with.

Not necessarily. The final configuration would simply be {...defaultConfig, ...userConfig} (maybe globalConfig, too, eg. ~/.config/appland/scanner.yaml). This is how most tools work and I think it's pretty expected.

— Reply to this email directly, view it on GitHub https://github.com/applandinc/scanner/pull/89#issuecomment-1022428427, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAVC6ZGH3GHEFC4OVM3JXDUYAV3JANCNFSM5MWDLHYQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

dividedmind commented 2 years ago

Ok so you’re fine with merging the configs you just want the default to be more explicit?

Yes! And also centralized and in the same format that the user config. Ideally it would also commented so that the user can copy it out and modify it as needed without needing to use an external reference. But even if the user doesn't do that it still serves as a nice reference in itself (of both the default configuration and what's possible to configure).