conda-forge / conda-smithy

The tool for managing conda-forge feedstocks.
https://conda-forge.org/
BSD 3-Clause "New" or "Revised" License
152 stars 183 forks source link

Support skipping noarch selector lints #2090

Closed isuruf closed 1 month ago

isuruf commented 1 month ago

When building python recipes that builds a python version specific extensions and noarch python variants, the lint can be wrong. This gives a way for maintainers to skip that lint.

Checklist

isuruf commented 1 month ago

cc @ocefpaf, @h-vetinari

h-vetinari commented 1 month ago

This was merged a bit quickly IMO (e.g. missing news). The functionality is good, though I would have avoided the redundant naming, i.e. the whole block

linter:
  skip:
    - lint_noarch_selectors

is under linter:, so prefixing the individual skips with lint_ seems pointless (and somewhat inconsistent) to me

isuruf commented 1 month ago

I was reading it as 'linter skip linting noarch selectors' and it seemed weird linter skip noarch selectors. What do you think?

I'll send a PR to add a news entry.

h-vetinari commented 1 month ago

I was reading it as 'linter skip linting noarch selectors' and it seemed weird linter skip noarch selectors. What do you think?

To me, the linter can (by definition/nomenclature/implementation) only lint, so the verb in the skip command is implicit IMO. We cannot tell it to skip anything else but linting.

If we think about how this might be extended in the future (taking some possible examples, not actually proposing those), I think that

linter:
  skip:
    - noarch_selectors
    - hints
    - stdlib
    - license 
    - source_sha

makes more sense than

linter:
  skip:
    - lint_noarch_selectors
    - lint_hints  # <-- this one especially 
    - lint_stdlib
    - lint_license 
    - lint_source_sha
isuruf commented 1 month ago

I was going to make a PR changing this, but it still felt weird to me. I don't understand the lint_hints comment. I was thinking

linter:
  skip:
    - lint_noarch_selectors
    - hint_pip_check
h-vetinari commented 1 month ago

It depends how granular we make the skips. Since they have no canonical names, I don't see another option than just documenting what skips we support and what they mean.

Why I thought about the hints is that - for example - a package may reasonably want to ignore the hint to replace matplotlib with matplotlib-base. But I wouldn't make the skip this granular, and just opt into skipping that part https://github.com/conda-forge/conda-smithy/blob/260544cc6a6fbfa8eebba8d74366f8932b452d85/conda_smithy/lint_recipe.py#L521-L524

No idea what we would name that, I had thought about just "all hints", but fair enough if that's too big a hammer.

I was thinking

linter:
  skip:
    - lint_noarch_selectors
    - hint_pip_check

At least the separation into lint_ and hint_ is a point that makes sense to me (even though there are opposing arguments still). In any case, this is hopefully going to be rare usage, so I'll drop this discussion, and you can leave things as they are.