Message ID | 20211008062821.1010967-1-eesposit@redhat.com |
---|---|
Headers | show |
Series | pylint: fix new errors and warnings in qemu-iotests | expand |
On 08.10.21 08:28, Emanuele Giuseppe Esposito wrote: > There are some warnings and errors that we either miss or > are new in pylint. Anyways, test 297 of qemu-iotests fails > because of that, so we need to fix it. > > All these fixes involve just indentation or additional spaces > added. > > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > --- > v2: > * temporarly enable and then disable "bad whitespace" error in image-fleecing > * better indentation for a fix in test 129 in patch one So the changes look good to me, but I can’t get my pylint to generate a bad-whitespace warning no matter how hard I try. (When you asked on IRC whether others see pylint warnings, I thought you meant the consider-using-f-string warnings that John disabled in 3765315d4c84f9c0799744f43a314169baaccc05.) I have pylint 2.11.1, which should be the newest version. So I tried to look around what might be the cause and found this: https://pylint.pycqa.org/en/latest/whatsnew/2.6.html – it seems that as of pylint 2.6, bad-whitespace warnings are no longer emitted. If that’s the reason why I can’t see the warning, then I think we should take only patch 1 (because it just makes sense), but skip patch 2. Hanna
On 11/10/2021 11:29, Hanna Reitz wrote: > On 08.10.21 08:28, Emanuele Giuseppe Esposito wrote: >> There are some warnings and errors that we either miss or >> are new in pylint. Anyways, test 297 of qemu-iotests fails >> because of that, so we need to fix it. >> >> All these fixes involve just indentation or additional spaces >> added. >> >> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> >> --- >> v2: >> * temporarly enable and then disable "bad whitespace" error in >> image-fleecing >> * better indentation for a fix in test 129 in patch one > > So the changes look good to me, but I can’t get my pylint to generate a > bad-whitespace warning no matter how hard I try. (When you asked on IRC > whether others see pylint warnings, I thought you meant the > consider-using-f-string warnings that John disabled in > 3765315d4c84f9c0799744f43a314169baaccc05.) > > I have pylint 2.11.1, which should be the newest version. So I tried to > look around what might be the cause and found this: > https://pylint.pycqa.org/en/latest/whatsnew/2.6.html – it seems that as > of pylint 2.6, bad-whitespace warnings are no longer emitted. If that’s > the reason why I can’t see the warning, then I think we should take only > patch 1 (because it just makes sense), but skip patch 2. > Yes you are right. I had 2.4.4, and now that I upgraded to 2.11.1 I don't see bad-whitespace errors anymore (actually pylint does not seem to complain at all). So I agree we can just take patch 1, as formatting is wrong anyways. Thank you, Emanuele
On 11.10.21 11:58, Emanuele Giuseppe Esposito wrote: > > > On 11/10/2021 11:29, Hanna Reitz wrote: >> On 08.10.21 08:28, Emanuele Giuseppe Esposito wrote: >>> There are some warnings and errors that we either miss or >>> are new in pylint. Anyways, test 297 of qemu-iotests fails >>> because of that, so we need to fix it. >>> >>> All these fixes involve just indentation or additional spaces >>> added. >>> >>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> >>> --- >>> v2: >>> * temporarly enable and then disable "bad whitespace" error in >>> image-fleecing >>> * better indentation for a fix in test 129 in patch one >> >> So the changes look good to me, but I can’t get my pylint to generate >> a bad-whitespace warning no matter how hard I try. (When you asked on >> IRC whether others see pylint warnings, I thought you meant the >> consider-using-f-string warnings that John disabled in >> 3765315d4c84f9c0799744f43a314169baaccc05.) >> >> I have pylint 2.11.1, which should be the newest version. So I tried >> to look around what might be the cause and found this: >> https://pylint.pycqa.org/en/latest/whatsnew/2.6.html – it seems that >> as of pylint 2.6, bad-whitespace warnings are no longer emitted. If >> that’s the reason why I can’t see the warning, then I think we should >> take only patch 1 (because it just makes sense), but skip patch 2. >> > > Yes you are right. I had 2.4.4, and now that I upgraded to 2.11.1 I > don't see bad-whitespace errors anymore (actually pylint does not seem > to complain at all). So I agree we can just take patch 1, as > formatting is wrong anyways. OK, thanks! I’ve applied patch 1 to my block branch: https://gitlab.com/hreitz/qemu/-/commits/block/ Hanna
On Mon, Oct 11, 2021 at 5:58 AM Emanuele Giuseppe Esposito < eesposit@redhat.com> wrote: > > > On 11/10/2021 11:29, Hanna Reitz wrote: > > On 08.10.21 08:28, Emanuele Giuseppe Esposito wrote: > >> There are some warnings and errors that we either miss or > >> are new in pylint. Anyways, test 297 of qemu-iotests fails > >> because of that, so we need to fix it. > >> > >> All these fixes involve just indentation or additional spaces > >> added. > >> > >> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > >> --- > >> v2: > >> * temporarly enable and then disable "bad whitespace" error in > >> image-fleecing > >> * better indentation for a fix in test 129 in patch one > > > > So the changes look good to me, but I can’t get my pylint to generate a > > bad-whitespace warning no matter how hard I try. (When you asked on IRC > > whether others see pylint warnings, I thought you meant the > > consider-using-f-string warnings that John disabled in > > 3765315d4c84f9c0799744f43a314169baaccc05.) > > > > I have pylint 2.11.1, which should be the newest version. So I tried to > > look around what might be the cause and found this: > > https://pylint.pycqa.org/en/latest/whatsnew/2.6.html – it seems that as > > of pylint 2.6, bad-whitespace warnings are no longer emitted. If that’s > > the reason why I can’t see the warning, then I think we should take only > > patch 1 (because it just makes sense), but skip patch 2. > > > > Yes you are right. I had 2.4.4, and now that I upgraded to 2.11.1 I > don't see bad-whitespace errors anymore (actually pylint does not seem > to complain at all). So I agree we can just take patch 1, as formatting > is wrong anyways. > > FWIW, the minimum version of pylint supported by the python/ tests is 2.8.0, for other reasons -- see commit b4d37d81. I no longer test or check for compatibility with older versions of pylint on any of our codebase. I've also authored a series to add iotest 297 to the Python CI directly, see https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg04329.html -- this gives a sense of "canonically supported linter versions" to that test. Currently, that support matrix is: Python 3.6 to Python 3.10 mypy >= 0.770 (Known to work with current latest, 0.910) pylint >= 2.8.0 (Known to work with current latest, 2.11.1) The "check-python-pipenv" test is the one Python CI test that will actually gate a build -- that test runs Python 3.6 with the oldest possible linter versions we support to ensure we don't accidentally start using new features. "check-python-tox" tests all python versions, 3.6 through 3.10, with bleeding edge packages. That CI test is "allowed to fail with a warning" and serves to alert me to new incompatible changes in updated Python dependencies. --js
There are some warnings and errors that we either miss or are new in pylint. Anyways, test 297 of qemu-iotests fails because of that, so we need to fix it. All these fixes involve just indentation or additional spaces added. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> --- v2: * temporarly enable and then disable "bad whitespace" error in image-fleecing * better indentation for a fix in test 129 in patch one Emanuele Giuseppe Esposito (2): pylint: fix errors and warnings generated by tests/qemu-iotests/297 qemu-iotests: fix image-fleecing pylint errors tests/qemu-iotests/129 | 18 +++++++++--------- tests/qemu-iotests/310 | 16 ++++++++-------- tests/qemu-iotests/check | 11 ++++++----- tests/qemu-iotests/iotests.py | 7 ++++--- tests/qemu-iotests/tests/image-fleecing | 8 ++++++-- 5 files changed, 33 insertions(+), 27 deletions(-)