[SERVER-68566] Disable pylinters on buildscripts/gdb/ Created: 04/Aug/22 Updated: 29/Oct/23 Resolved: 05/Aug/22 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 6.1.0-rc0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Daniel Gottlieb (Inactive) | Assignee: | Daniel Gottlieb (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||
| Description |
|
Many of the linters are low-value, such as the docstring requirement. It's almost universally elided with a docstring that mimics the function name, e.g:
I think we can consider turning some* pylinters on for our gdb scripts, but right now we're creating a barrier to entry on a part of the codebase where we should be sharing more tools rather than using a poor metric for code quality that has veto rights. Most scripts we want to add are already being shared, but outside of the repository. The utility is apparent. |
| Comments |
| Comment by Daniel Gottlieb (Inactive) [ 05/Aug/22 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I've closed this ticket, but feel free to re-open if you just want to piggy-back on it for re-adding the better curated rules. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Daniel Gottlieb (Inactive) [ 05/Aug/22 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Yes, I'm aware. That's what I use today. But that's not good enough UX. Today many engineers are using some version of "format my code on save such that lint checks will pass". For server engineers working only on c++ and js files, that's easy (pseudocode-y):
But for engineers that may touch python files under the jurisdiction of pylint, we now need:
If that logic to determine which file maps to which "formatter" were baked into some new published shell script, engineers could just have:
Modulo some portability improvements. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Ryan Egesdahl (Inactive) [ 05/Aug/22 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
daniel.gottlieb@mongodb.com Such a thing does exist already in the repo as buildscripts/pylinters.py lint-all, and you can use fix to automatically fix them if you want. The commit queue uses lint-patch. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Daniel Gottlieb (Inactive) [ 05/Aug/22 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Using the following grep as a seed of warnings that might be worth disabling:
I've written a script to parse/count them up:
I don't know if that passes our pylint, but in the spirit of investigation, I've written that to be intentionally readable unfriendly The results:
I'm going to blindly recommend everything >= 10 also be suppressed.
To be clear, I think the highest value thing is a single script that can correctly identify and run the proper in-place format tool for any file in the mongo repository that is linted against. Today we use clang-format works for our c++ and js code with a single invocation. But fixing python files in-place requires a separate invocation. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Alex Neben [ 05/Aug/22 ] | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
From our discussion we are going to disable the following python warnings We are also going to create and/or provide a way for running python formatting/linting as a script/pluggable into vs code |