Is your feature request related to a problem? Please describe.
In #11099 @JulienCochuyt added sconscript files to the list of files which are checked for compliance with NVDA's coding style by the Flake8 Linter. While Sconscripts are, for the most part, standard Python files and general linting rules apply for them there are two cases of F821 (undefined names errors) which were confusing for developers. Due to that Linting was disabled for them in #11817.
Describe the solution you'd like
I believe having these files linted was pretty beneficial - while they're not changed every day sticking to an unified coding style is important IMHO. I fell most of the confusion around undefined names errors can be eliminated by documenting what style should be followed for them. I propose, and if agreed up on will submit a PR, documenting the following rules:
- For built-in Scons function it is sufficient to import them again from the Scons package so for example if Flake8 complains about
Import being undefined we can just do:
from SCons.Script import Import
Since these names are already in globals there is no performance penalty caused by importing them explicitly.
- The second case is trickier as Flake8 also complains about names imported using
Import function from higher level sconscripts, but since Scons inserts them to globals when using Import we can simply do:
env = globals["env"]
Additional advantage of assigning them to variables explicitly is that we can use type annotations for them I believe.
Describe alternatives you've considered
Several comes to my mint:
- Don't Lint sconscripts at all
- Ignore every F821 error with a NOQA - would get pretty messy after a while _ might potentially hide other undefined names in the same line.
- We can also add second Flake8 config just for Sconscripts in which we would ignore F821 by default.
Additional context
None
Is your feature request related to a problem? Please describe.
In #11099 @JulienCochuyt added sconscript files to the list of files which are checked for compliance with NVDA's coding style by the Flake8 Linter. While Sconscripts are, for the most part, standard Python files and general linting rules apply for them there are two cases of F821 (undefined names errors) which were confusing for developers. Due to that Linting was disabled for them in #11817.
Describe the solution you'd like
I believe having these files linted was pretty beneficial - while they're not changed every day sticking to an unified coding style is important IMHO. I fell most of the confusion around undefined names errors can be eliminated by documenting what style should be followed for them. I propose, and if agreed up on will submit a PR, documenting the following rules:
Importbeing undefined we can just do:from SCons.Script import ImportSince these names are already in globals there is no performance penalty caused by importing them explicitly.
Importfunction from higher level sconscripts, but since Scons inserts them to globals when usingImportwe can simply do:env = globals["env"]Additional advantage of assigning them to variables explicitly is that we can use type annotations for them I believe.
Describe alternatives you've considered
Several comes to my mint:
Additional context
None