Zgłaszanie wszystkich wyłączonych ostrzeżeń w pull requeście

[cameel]

Często zdarza mi się napotkać w kodzie miejsca, gdzie zostaje wyciszony warning, mimo, że wskazuje on na rzeczywisty problem. Moim zdaniem musimy podjąć jakieś kroki aby zapobiec takim sytuacjom. Są sytuacje, kiedy rzeczywiście warning jest fałszywym alarmem ale powinny być one rzadkie. Jeżeli zdarzają się często, to lepiej wyłączyć część sprawdzeń w pylintrc niż przyzwyczajać się do machinalnego wyłączania wszystkiego.

Na przykład bardzo częstą sytuacją jest ostrzeżenie no-member z pylinta spowodowane tym, że klasa definiuje jakąś funkcję lub pole w w zbyt dynamiczny sposób aby łatwo wykryć to podczas statycznej analizy. Zamiast umieszczać na każdej linijce # pylint: disable=no-member znacznie lepiej w pliku pylintrc dodać tę klasę do ignored-classes. Wyłączy to sprawdzanie tylko dla niej.

Aby zmienić tę sytuację proponuję wprowadzić następującą zmianę w naszym workflow:

Jeżeli w ramach pull requestu wyciszane są jakiekolwiek ostrzeżenia pythona, linta, checkera typów, itp., w pull requeście powinien znaleźć się komentarz z listą wszystkich miejsc gdzie coś zostało wyciszone, każde z krótkim uzasadnieniem. Wszystkie wyciszenia muszą zostać potwierdzone i zaakceptowane przez senior developera (który nie jest autorem pull requestu).

Zmusi to do zastanowienia się nad przyczyną i pozwoli też innym zasugerować lepsze rozwiązanie. Sprawi też, że zwykłe wyłączenie warninga nie będzie najłatwiejszym wyjściem z sytuacji. Zwłaszcza w sytuacjach kiedy warningów jest bardzo dużo - nawet jeżeli wyłączenie jest słuszne, to gdy jest potrzebne zbyt często, należy szukać innego wyjścia z sytuacji.