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
Allow disable reporting to be controlled from the config file #5126
Conversation
c0b0508
to
e7a29c5
Compare
e7a29c5
to
a46897f
Compare
Co-authored-by: Jennifer Thakar <jathak@google.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nex3 Thank you. It's amazing to see this go in. As you said, it's the first step towards adding the overrides
configuration property (which opens up so many possibilities). It's also the first step towards being able to configure everything from within the configuration object, rather than a hodgepodge of properties, parameters and flags.
I've requested some minor documentation changes.
At the start of last year we restructured and rewrote the majority of the docs. This included removing a lot of duplication where API parameters, CLI flags and configuration properties had fallen out of sync.
I'm aware this pull request reintroduces duplication, but I suggest we roll with it for now. I'll also merge in #5005, which duplicates the CLI flags. Once everything is configurable within the configuration object, I think we'll be better positioned to revisit the documentation and find a structure that both avoids duplication and is intuitive for users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the requested changes.
LGTM!
|
This will also make it easier to control disables on a file-by-file basis
once #3128 is implemented. It will also make it possible to add
finer-grained configuration to these rules in the future.
Closes #3858