yamllint icon indicating copy to clipboard operation
yamllint copied to clipboard

Consider changing `truthy` linter's `check-keys` default

Open gravieure opened this issue 1 year ago • 6 comments

The default configuration for the truthy linter applies it to keys as well as values. The syntax for GitHub Actions definitions uses an on key to determine what event the workflow applies to.

The upshot is that, out of the box, yamllint will blow up on any YAML document configuring a GitHub Action. This certainly can be fixed by disabling check-keys, but the presence of that option is non-obvious.

Given that GitHub Actions are a very common YAML usecase, do you think it'd be reasonable to disable this linter's check-keys option by default?

gravieure avatar Apr 04 '23 18:04 gravieure

Hello, is it a duplicate of #540, #430, #344, #232, #247, #158 and others?

adrienverge avatar Apr 05 '23 06:04 adrienverge

No; those all assume that the behavior is a bug needing to be fixed. I understand that it is not a bug, but an option which is enabled by default. My question is: would it make sense to change that default? The numerous issues filed on this subject are strong evidence that the default is confusing.

If changing the default is disagreeable, what do you think about the truthy linter printing some info about the check-keys option in this scenario? I think that would also reduce confusion and duplicate bug reports, since people would be guided to the right option without needing to file an issue about it.

gravieure avatar Apr 05 '23 14:04 gravieure

I was thinking to suggest using a %YAML 1.2 tag to automatically disable check-keys. But then again, found out that pyyaml (still!) doesn't support yaml 1.2.

Switching to ruamel.yaml might be an option, but that's for another discussion.

SubaruArai avatar Aug 09 '23 03:08 SubaruArai

I think this is very reasonable, either disable check-keys or disable it for files inside of .github/workflows

777arc avatar Aug 11 '23 06:08 777arc

TL;DR: For those seeking a quick-win to silent the error on GitHub Actions, here are 4 solutions: - use config option rules: {truthy: {check-keys: false}} - use config option rules: {truthy: {allowed-keys: [on, …]}} - disable the check on the specific line with # yamllint disable-line rule:truthy - quote the on key, e.g. "on"

No; those all assume that the behavior is a bug needing to be fixed. I understand that it is not a bug, but an option which is enabled by default. My question is: would it make sense to change that default? The numerous issues filed on this subject are strong evidence that the default is confusing.

I understand, but changing a default configuration in yamllint would break behavior in processes (e.g. continuous integration) for many users and companies. It is not an option to consider.

If changing the default is disagreeable, what do you think about the truthy linter printing some info about the check-keys option in this scenario? I think that would also reduce confusion and duplicate bug reports, since people would be guided to the right option without needing to file an issue about it.

This is interesting. But if I understand correctly, your proposition would produce extra "human-friendly" output amongst yamllint regular output (which has a constant pattern). I fear that this also would break tools relying on yamllint's output.

adrienverge avatar Aug 11 '23 07:08 adrienverge

I'd say we should rather lobby Github to update their workflows, to one of:

Suggesting a quoted 'on' or (even better) always interpret the workflow files as YAML 1.1 (maybe that's already what they're doing).

A temporary option for yamllint users:

  # .yamllint

  # don't use YAML's yes/no as boolean values (common cause of bugs)
  truthy:
    level: error

    # github action workflows use the key 'on' without quotes
    ignore: |
      .github/workflows/*.yml

sandstrom avatar Nov 16 '23 14:11 sandstrom