Re: [PATCH v1 06/10] kconfig-checker: Split schema to support component reuse

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Low] kconfig-checker: stale schema path in
  Documentation/tools/kconfig_checker.rst
- [Low] kconfig-checker: missing type: ignore on Draft7Validator import
- [Low] kconfig-checker: unused Schema.as_dict() method left behind in
  check.py
--

commit e95d8c61bd1b396a43ce4115372f7685c287a202
Author: Ahmed S. Darwish <darwi@xxxxxxxxxxxxx>

kconfig-checker: Split schema to support component reuse

This commit moves the common rule schema into schemas/base.yml, adds
schemas/rules.yml for complete rule files, and updates check.py to resolve
local file:// schema references using Draft7Validator.

> diff --git a/tools/kconfig-checker/check.py b/tools/kconfig-checker/check.py
> index ef49c28b9dab6..4f79ec487da79 100755
> --- a/tools/kconfig-checker/check.py
> +++ b/tools/kconfig-checker/check.py
> @@ -80,6 +80,8 @@ except ModuleNotFoundError as e:
>  # Actual script:
>  #
>  
> +from jsonschema import Draft7Validator

[Severity: Low]
Does this import need a "# type: ignore" comment to match the earlier
"import jsonschema" statement?  Without it, running "mypy --strict
--no-warn-return-any check.py" as documented in the file header fails with
a missing-stubs error (or fails on the earlier "# type: ignore" when
types-jsonschema is installed).

[ ... ]

>      def as_dict(self) -> YamlDict:
>          return self.schema
> @@ -273,7 +308,7 @@ class RulesFile:
>  
>      def __init__(self, yaml_f: Path, schema: Schema):
>          self.yaml = Helpers.load_yaml(yaml_f)
> -        Helpers.validate_yaml(self.yaml, schema.as_dict(), yaml_f.name)
> +        Helpers.validate_yaml(self.yaml, schema.validator, yaml_f.name)

[Severity: Low]
This isn't a bug, but now that RulesFile.__init__() and
Helpers.validate_yaml() take schema.validator instead of schema.as_dict(),
Schema.as_dict() has no remaining callers in this patch or later in the
series and could be removed.

[ ... ]

> @@ -533,6 +568,7 @@ def parse_args(rules: list[str]) -> argparse.Namespace:
>  def main() -> None:
>      base_dir = Path(__file__).resolve().parent
>      rules_dir = base_dir / 'rules'
> +    schema_dir = base_dir / 'schemas'
>  
>      rules_l = sorted(path.stem for path in rules_dir.glob('*.yml'))
>      args = parse_args(rules_l)
> @@ -544,7 +580,7 @@ def main() -> None:
>      kconfig_f = args.config.resolve()
>      kconfig = KernelConfig(kconfig_f)
>  
> -    schema_f = base_dir / 'schema.yml'
> +    schema_f = schema_dir / 'rules.yml'

[Severity: Low]
Should Documentation/tools/kconfig_checker.rst also be updated here now
that tools/kconfig-checker/schema.yml has moved to
tools/kconfig-checker/schemas/base.yml and
tools/kconfig-checker/schemas/rules.yml?  Later commit "Documentation:
Describe kconfig-checker overlays" updates that section to reference
tools/kconfig-checker/base.yml and tools/kconfig-checker/rules.yml,
omitting the schemas/ directory component.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106091922.7560-1-darwi@xxxxxxxxxxxxx?part=6