Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configuration of truthy keys #344

Closed
mondeja opened this issue Nov 26, 2020 · 10 comments
Closed

Allow configuration of truthy keys #344

mondeja opened this issue Nov 26, 2020 · 10 comments

Comments

@mondeja
Copy link

mondeja commented Nov 26, 2020

Linting Github Actions configuration files I'm getting the warning truthy value should be one of [false, true] (truthy) with on keys. For discussion, is not this default behaviour too much intrusive?

The solution would be set check-keys: false in truthy configuration, but this removes the verification for all keys. Maybe an allowed-keys configuration can help in this case?

Ignore solution if someone needs it:

rules:
  truthy:
    allowed-values:
      - 'true'
      - 'false'
    check-keys: true
    ignore: |
      .github/workflows
@adrienverge
Copy link
Owner

Hello,

The solution would be set check-keys: false in truthy configuration, but this removes the verification for all keys.

What do you mean exactly with "but this removes the verification for all keys"? Can you give an example of what values you would put in this allowed-keys list?

@mondeja
Copy link
Author

mondeja commented Nov 26, 2020

What do you mean exactly with "but this removes the verification for all keys"?

If I've another key, for example On in the file would not be checked, not?

Can you give an example of what values you would put in this allowed-keys list?

allowed-keys:
  - on

So with this On would be treated as warning but not on.

@adrienverge
Copy link
Owner

If I've another key, for example On in the file would not be checked, not?

Sure, but why would you want to detect and report On (and other possibly truthy values), if you're not interested by reporting on? Why would you want to detect and report On, but not Hey or Álvaro?

If I understand your use-case, you either want to:

  • use the rule truthy with {check-keys: false} (keys can be whatever they want),
  • or have a new rule (not truthy) that would check keys and forbid them if they match a list.

@mondeja
Copy link
Author

mondeja commented Nov 26, 2020

Sorry, but I don't understand your point. What is the solution that you propose for Github Actions configuration files linting? This next?

rules:
  truthy:
    allowed-values:
      - 'true'
      - 'false'
    check-keys: true
    ignore: |
      .github/workflows

To ignore the file?

Given next Github Actions configuration file, if I remove the ignore I receive the error from yamllint:

name: pre-commit

on:
  pull_request:
  push:
    branches:
      - master

jobs:
  pre-commit:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - name: Set up Python
        uses: actions/setup-python@v2
        with:
          python-version: 3.x

Next error:

./.github/workflows/pre-commit.yml
  3:1       error    truthy value should be one of [false, true]  (truthy)

Is the on working for keys or values? Maybe I don't understand the rule at all.

@adrienverge
Copy link
Owner

adrienverge commented Nov 26, 2020

If you want to use on as a key in your YAML file, you need to disable the truthy rule or use check-keys: false. From what you post, it seems like you don't want check-keys: true.

@mondeja
Copy link
Author

mondeja commented Nov 26, 2020

Wouldn't it be helpful if you could define the keys for which the rule works? I want check-keys: true but I don't want to use this rule for on keys.

@adrienverge
Copy link
Owner

I don't think so, but maybe you have a specific use-case or example?

Why would you want to forbid On to be a key, but not on?

@mondeja
Copy link
Author

mondeja commented Nov 26, 2020

This, for example:

name: whatever

on:
  pull_request:
  push:
    branches:
      - master

jobs:
  whatever:
    runs-on: ubuntu-latest
    steps:
      - uses: some-new-action-that/someone-implements-in-the-future@v88
        with:
          yes: false

Here yes must be linted but on simply ignored.

@mondeja
Copy link
Author

mondeja commented Nov 26, 2020

And why not add fired to be a truthy key that must be checked, or released, or deprecated? If this rule is so arbitrary, at least that keys can be configured.

@mondeja mondeja changed the title Truthy default keys too much intrusive? Allow configuration of truthy keys Nov 26, 2020
@adrienverge
Copy link
Owner

And why not add fired to be a truthy key that must be checked, or released, or deprecated? If this rule is so arbitrary, at least that keys can be configured.

Sure. As I said here, what you're talking about is a new rule (not truthy) that would check keys, and report is their value is forbidden.

@mondeja mondeja closed this as completed Nov 26, 2020
adrienverge added a commit that referenced this issue Jan 31, 2024
Specification of YAML ≤ 1.1 has 18 boolean values:

    true  | True  | TRUE | false | False | FALSE
    yes   | Yes   | YES  | no    | No    | NO
    on    | On    | ON   | off   | Off   | OFF

Whereas YAML 1.2 spec recognizes only 6:

    true  | True  | TRUE | false | False | FALSE

For documents that explicit state their YAML spec version at the top of
the document, let's adapt the list of forbidden values.

In the future, we should:
- implement a configuration option to declare the default YAML spec
  version, e.g. `default-yaml-spec-version: 1.2`,
- consider making 1.2 the default in a future release (this would be a
  slight breaking change, but yamllint always tried to be
  1.2-compatible).
- consider adapting yamllint to other 1.1 vs. 1.2 differences [^2].

Solves: #587

Related to: #559 #540 #430 #344 #247 #232 #158

[^1]: https://yaml.org/spec/1.2.2/#1032-tag-resolution
[^2]: https://yaml.org/spec/1.2.2/ext/changes/#changes-in-version-12-revision-120-2009-07-21
adrienverge added a commit that referenced this issue Jan 31, 2024
Specification of YAML ≤ 1.1 has 18 boolean values:

    true  | True  | TRUE | false | False | FALSE
    yes   | Yes   | YES  | no    | No    | NO
    on    | On    | ON   | off   | Off   | OFF

Whereas YAML 1.2 spec recognizes only 6 [^1]:

    true  | True  | TRUE | false | False | FALSE

For documents that explicit state their YAML spec version at the top of
the document, let's adapt the list of forbidden values.

In the future, we should:
- implement a configuration option to declare the default YAML spec
  version, e.g. `default-yaml-spec-version: 1.2`,
- consider making 1.2 the default in a future release (this would be a
  slight breaking change, but yamllint always tried to be
  1.2-compatible).
- consider adapting yamllint to other 1.1 vs. 1.2 differences [^2].

Solves: #587

Related to: #559 #540 #430 #344 #247 #232 #158

[^1]: https://yaml.org/spec/1.2.2/#1032-tag-resolution
[^2]: https://yaml.org/spec/1.2.2/ext/changes/#changes-in-version-12-revision-120-2009-07-21
adrienverge added a commit that referenced this issue Feb 4, 2024
Specification of YAML ≤ 1.1 has 22 boolean values:

    y     | Y            | n     | N
    yes   | Yes   | YES  | no    | No    | NO
    true  | True  | TRUE | false | False | FALSE
    on    | On    | ON   | off   | Off   | OFF

Whereas YAML 1.2 spec recognizes only 6 [^1]:

    true  | True  | TRUE | false | False | FALSE

For documents that explicit state their YAML spec version at the top of
the document, let's adapt the list of forbidden values.

In the future, we should:
- implement a configuration option to declare the default YAML spec
  version, e.g. `default-yaml-spec-version: 1.2`,
- consider making 1.2 the default in a future release (this would be a
  slight breaking change, but yamllint always tried to be
  1.2-compatible).
- consider adapting yamllint to other 1.1 vs. 1.2 differences [^2].

Solves: #587

Related to: #559 #540 #430 #344 #247 #232 #158

[^1]: https://yaml.org/spec/1.2.2/#1032-tag-resolution
[^2]: https://yaml.org/spec/1.2.2/ext/changes/#changes-in-version-12-revision-120-2009-07-21
adrienverge added a commit that referenced this issue Feb 6, 2024
Specification of YAML ≤ 1.1 has 22 boolean values:

    y     | Y            | n     | N
    yes   | Yes   | YES  | no    | No    | NO
    true  | True  | TRUE | false | False | FALSE
    on    | On    | ON   | off   | Off   | OFF

Whereas YAML 1.2 spec recognizes only 6 [^1]:

    true  | True  | TRUE | false | False | FALSE

For documents that explicit state their YAML spec version at the top of
the document, let's adapt the list of forbidden values.

In the future, we should:
- implement a configuration option to declare the default YAML spec
  version, e.g. `default-yaml-spec-version: 1.2`,
- consider making 1.2 the default in a future release (this would be a
  slight breaking change, but yamllint always tried to be
  1.2-compatible).
- consider adapting yamllint to other 1.1 vs. 1.2 differences [^2].

Solves: #587

Related to: #559 #540 #430 #344 #247 #232 #158

[^1]: https://yaml.org/spec/1.2.2/#1032-tag-resolution
[^2]: https://yaml.org/spec/1.2.2/ext/changes/#changes-in-version-12-revision-120-2009-07-21
adrienverge added a commit that referenced this issue Feb 6, 2024
Specification of YAML ≤ 1.1 has 22 boolean values:

    y     | Y            | n     | N
    yes   | Yes   | YES  | no    | No    | NO
    true  | True  | TRUE | false | False | FALSE
    on    | On    | ON   | off   | Off   | OFF

Whereas YAML 1.2 spec recognizes only 6 [^1]:

    true  | True  | TRUE | false | False | FALSE

For documents that explicit state their YAML spec version at the top of
the document, let's adapt the list of forbidden values.

In the future, we should:
- implement a configuration option to declare the default YAML spec
  version, e.g. `default-yaml-spec-version: 1.2`,
- consider making 1.2 the default in a future release (this would be a
  slight breaking change, but yamllint always tried to be
  1.2-compatible).
- consider adapting yamllint to other 1.1 vs. 1.2 differences [^2].

Solves: #587

Related to: #559 #540 #430 #344 #247 #232 #158

[^1]: https://yaml.org/spec/1.2.2/#1032-tag-resolution
[^2]: https://yaml.org/spec/1.2.2/ext/changes/#changes-in-version-12-revision-120-2009-07-21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants