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

ci(yamllint): add rule empty-values & use new yaml-files setting #228

Merged
merged 1 commit into from
Sep 14, 2019

Conversation

myii
Copy link
Member

@myii myii commented Sep 10, 2019

mysql-formula$ yamllint -s .
./mysql/osfamilymap.yaml
  46:14     error    empty value in block mapping  (empty-values)
  66:14     error    empty value in block mapping  (empty-values)
  67:14     error    empty value in block mapping  (empty-values)
  68:16     error    empty value in block mapping  (empty-values)
  69:17     error    empty value in block mapping  (empty-values)
  70:16     error    empty value in block mapping  (empty-values)
  87:7      error    empty value in block mapping  (empty-values)
  216:14    error    empty value in block mapping  (empty-values)
  217:17    error    empty value in block mapping  (empty-values)

./mysql/defaults.yaml
  29:10     error    empty value in block mapping  (empty-values)
  30:11     error    empty value in block mapping  (empty-values)

@myii myii changed the title ci(yamllint): add rule empty-values & use new yaml-files setting WIP: ci(yamllint): add rule empty-values & use new yaml-files setting Sep 10, 2019
@myii
Copy link
Member Author

myii commented Sep 10, 2019

Need to fix before merging:

        yamllint:
          rules:
            empty-values:
              ignore:
                - mysql/defaults.yaml
                - mysql/osfamilymap.yaml

@myii
Copy link
Member Author

myii commented Sep 10, 2019

@noelmcloughlin Initial cross-reference: saltstack-formulas/template-formula#164.

I'm requesting your review here because I'm not happy with the fixes being applied. The empty values that are currently there translate to null, which I've explicitly set using ~. That doesn't seem right in the context, though. What are these values supposed to be? Or should those entries be commented out?

The defaults.yaml fixes are fine, it's the osfamilymap.yaml values that I'm asking about.

@myii myii force-pushed the chore/standardise-structure branch from b87a28d to 9775305 Compare September 14, 2019 22:24
* Semi-automated using myii/ssf-formula#27
* Fix (or ignore) errors shown below:

```bash
mysql-formula$ yamllint -s .
./mysql/osfamilymap.yaml
  46:14     error    empty value in block mapping  (empty-values)
  66:14     error    empty value in block mapping  (empty-values)
  67:14     error    empty value in block mapping  (empty-values)
  68:16     error    empty value in block mapping  (empty-values)
  69:17     error    empty value in block mapping  (empty-values)
  70:16     error    empty value in block mapping  (empty-values)
  87:7      error    empty value in block mapping  (empty-values)
  216:14    error    empty value in block mapping  (empty-values)
  217:17    error    empty value in block mapping  (empty-values)

./mysql/defaults.yaml
  29:10     error    empty value in block mapping  (empty-values)
  30:11     error    empty value in block mapping  (empty-values)
```
@myii myii force-pushed the chore/standardise-structure branch from 9775305 to 2322ff6 Compare September 14, 2019 22:25
@myii myii changed the title WIP: ci(yamllint): add rule empty-values & use new yaml-files setting ci(yamllint): add rule empty-values & use new yaml-files setting Sep 14, 2019
@myii myii merged commit 70e5b43 into saltstack-formulas:master Sep 14, 2019
@myii
Copy link
Member Author

myii commented Sep 14, 2019

Used inline yamllint ignores to get around this issue in the meantime. Transferred this to issue #229. Self-merging alongside all of the other formulas that have already incorporated this.

@saltstack-formulas-travis

🎉 This PR is included in version 0.52.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants