add `--max-error-percentage` flags

PyCQA / pydocstyle

docstring style checker

http://pydocstyle.org

MIT License

1.11k stars 188 forks source link

add `--max-error-percentage` flags #597

Closed mashanz closed 2 years ago

mashanz commented 2 years ago

Thanks for submitting a PR!

Please make sure to check for the following items:

[ ] Add unit tests and integration tests where applicable.
If you've added an error code or changed an error code behavior, you should probably add or change a test case file under tests/test_cases/ and add it to the list under tests/test_definitions.py.
If you've added or changed a command line option, you should probably add or change a test in tests/test_integration.py.
[ ] Add a line to the release notes (docs/release_notes.rst) under "Current Development Version".
Make sure to include the PR number after you open and get one.

Please don't get discouraged as it may take a while to get a review.

sigmavirus24 commented 2 years ago

What issue is this related to? Why would someone want to set a percentage of failures in order to succeed? The code is not clear on what it's trying to do and I don't see any prior discussion of this

mashanz commented 2 years ago

Hi @sigmavirus24 , There is rule on my company to create documentation on software that we are building, and we check the documentation coverage using pydocstyle and running it on CI/CD pipeline.

but because our app is to huge but less documented, we set at least 50% the file is fully documented. and we will gradually decrease the error percentage until the software 100% fully documented overtime. also, we set failure on pipeline if the new PR code doesn't have those minimum percentage documentation (current app + new pr documentation coverage) to prevent PR being merged with main branch.

because pydocstyle didn't have those use case to cover this specific needs, we introduce --max-error-percentage on pydocstyle to cover this use case.

I hope this clarify this PR. Thanks.

sigmavirus24 commented 2 years ago

It does clarify, thank you. I would structure this differently, personally and even then I'm not convinced it belongs in this tool. If it were to live here, it should look more like coverage.py and its flags.

mashanz commented 2 years ago

Thanks for the advice. Will update this PR.

sigmavirus24 commented 2 years ago

Actually, I wouldn't even integrate that here because it's already covered by https://github.com/econchick/interrogate

mashanz commented 2 years ago

Actually, I wouldn't even integrate that here because it's already covered by https://github.com/econchick/interrogate

oh, That package is new to me, thanks @sigmavirus24 for the information. It's very helpful. I think we can close this PR and I will use that tools on our CI/CD's