pylint and shellcheck

Hi!

It's merge window time so I have a bit of time to catch up on random things. I added shellcheck, yamllint and pylint:
https://github.com/linux-netdev/nipa/commit/c0fe53ae533d19c19d2e00955403fb57c3679084
https://github.com/linux-netdev/nipa/commit/255ee0295a096ee7096bebd9d640388acc590da0
https://github.com/linux-netdev/nipa/commit/54e060c9094e33bffe356b5d3e25853e22235d49
to the netdev patchwork checks.

They will likely be pretty noisy so please take them with a grain of salt (pretty much like checkpatch). Using the NIPA scripts from the commits above could be useful to find the delta of new warnings, since there will be quite a few existing ones.

I suspect as we get more experience we will find the warning types to disable, and we will drive the number of existing errors down to make checking for new ones less of a pain. As I said, for now please don't take these checks failing at face value.

Hi!

It's merge window time so I have a bit of time to catch up on random things. I added shellcheck, yamllint and pylint:
https://github.com/linux-netdev/nipa/commit/c0fe53ae533d19c19d2e00955403fb57c3679084
https://github.com/linux-netdev/nipa/commit/255ee0295a096ee7096bebd9d640388acc590da0
https://github.com/linux-netdev/nipa/commit/54e060c9094e33bffe356b5d3e25853e22235d49
to the netdev patchwork checks.

They will likely be pretty noisy so please take them with a grain of salt (pretty much like checkpatch). Using the NIPA scripts from the commits above could be useful to find the delta of new warnings, since there will be quite a few existing ones.

I suspect as we get more experience we will find the warning types to disable, and we will drive the number of existing errors down to make checking for new ones less of a pain. As I said, for now please don't take these checks failing at face value.

Quoted 14 lines · Jakub Kicinski

Hi!

It's merge window time so I have a bit of time to catch up on random things. I added shellcheck, yamllint and pylint:
https://github.com/linux-netdev/nipa/commit/c0fe53ae533d19c19d2e00955403fb57c3679084
https://github.com/linux-netdev/nipa/commit/255ee0295a096ee7096bebd9d640388acc590da0
https://github.com/linux-netdev/nipa/commit/54e060c9094e33bffe356b5d3e25853e22235d49
to the netdev patchwork checks.

They will likely be pretty noisy so please take them with a grain of salt (pretty much like checkpatch). Using the NIPA scripts from the commits above could be useful to find the delta of new warnings, since there will be quite a few existing ones.

Quoted 4 lines

I suspect as we get more experience we will find the warning types to disable, and we will drive the number of existing errors down to make checking for new ones less of a pain. As I said, for now please don't take these checks failing at face value.

Thanks Jakub,

I agree this is a good step.

Anecdotally, my feeling from running shellcheck over patches for a little while now is that the feedback it gives mainly relates to stricter coding practices which aren't generally followed. And yet the scripts seem to run reliably in the environments they are intended to run in.

So I'll be interested to see if we end up go for some mix of disabling warnings and updating (creating!) our preferred coding style for shell scripts.

</2c>

This is a possible config for yamllint:

extends: default
rules:
document-start: disable
brackets:
max-spaces-inside: 1
comments:
min-spaces-from-content: 1
line-length:

    max: 96

Quoted 11 lines · On Wed, 04 Jun 2025 10:41:14 +0100 Donald Hunter wrote:

This is a possible config for yamllint:

extends: default
rules:
document-start: disable
brackets:
max-spaces-inside: 1
comments:
min-spaces-from-content: 1
line-length:

    max: 96

This fits our current style pretty nicely!

One concern I have is that yamllint walks down the filesystem CWD down to root or home dir. So if we put this in Documentation/netlink/.yamllint people running yamllint from main dir:

 $ yamllint Documentation/netlink/specs/netdev.yaml

will be given incorrect warnings, no? Is there a workaround?

Quoted 19 lines · Jakub Kicinski

Quoted 11 lines · On Wed, 04 Jun 2025 10:41:14 +0100 Donald Hunter wrote:

This is a possible config for yamllint:

extends: default
rules:
document-start: disable
brackets:
max-spaces-inside: 1
comments:
min-spaces-from-content: 1
line-length:

    max: 96  

This fits our current style pretty nicely!

One concern I have is that yamllint walks down the filesystem CWD down to root or home dir. So if we put this in Documentation/netlink/.yamllint people running yamllint from main dir:

Quoted 3 lines

 $ yamllint Documentation/netlink/specs/netdev.yaml

will be given incorrect warnings, no? Is there a workaround?

I don't see a workaround without some kind of wrapper.

Maybe just add a makefile? Looks like that was the approach taken for Documentation/devicetree/bindings

Quoted 25 lines · On Thu, 05 Jun 2025 10:02:10 +0100 Donald Hunter wrote:

Quoted 22 lines · Jakub Kicinski

Quoted 11 lines · On Wed, 04 Jun 2025 10:41:14 +0100 Donald Hunter wrote:

This is a possible config for yamllint:

extends: default
rules:
document-start: disable
brackets:
max-spaces-inside: 1
comments:
min-spaces-from-content: 1
line-length:

    max: 96  

This fits our current style pretty nicely!

One concern I have is that yamllint walks down the filesystem CWD down to root or home dir. So if we put this in Documentation/netlink/.yamllint people running yamllint from main dir:

 $ yamllint Documentation/netlink/specs/netdev.yaml

will be given incorrect warnings, no? Is there a workaround?

Quoted 4 lines

I don't see a workaround without some kind of wrapper.

Maybe just add a makefile? Looks like that was the approach taken for Documentation/devicetree/bindings

If we can live with the document-start annoyance I was wondering if we can stick to the "default" as is? We can fix existing docs slowly, the patchwork script will ignore pre-existing warnings.

I could have patches ready for the merge window opening. What's best, a patch per type of error, or some other split?