Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

Linting #23

Closed
2 of 3 tasks
viovanov opened this issue Sep 17, 2019 · 7 comments · Fixed by #224
Closed
2 of 3 tasks

Linting #23

viovanov opened this issue Sep 17, 2019 · 7 comments · Fixed by #224
Assignees
Labels
Priority: Medium SUSE SUSE is pursuing a solution Type: CI Issues for working on the project's CI pipelines
Milestone

Comments

@viovanov
Copy link
Member

viovanov commented Sep 17, 2019

Sources should be linted

  • for yaml files
  • for scripts
  • for helm templates after helm building the helm chart, with helm lint
@f0rmiga
Copy link
Member

f0rmiga commented Sep 27, 2019

We already have linting for scripts using shellcheck. Shellcheck dependency is also managed by Bazel, so no need to manually manage its version on each developer environment.

@viovanov viovanov transferred this issue from SUSE/scf Oct 18, 2019
@viovanov
Copy link
Member Author

viovanov commented Oct 30, 2019

@viovanov viovanov added the Type: CI Issues for working on the project's CI pipelines label Nov 7, 2019
@f0rmiga f0rmiga mentioned this issue Nov 8, 2019
7 tasks
@fargozhu fargozhu added the SUSE SUSE is pursuing a solution label Nov 12, 2019
@fargozhu fargozhu added this to the 0.1.x milestone Nov 12, 2019
@viccuad
Copy link
Member

viccuad commented Nov 13, 2019

An example of using yamllint and shellcheck, skipping some folders, and in posix standard:
https://github.com/SUSE/catapult/blob/master/tests/lint.sh

@viccuad
Copy link
Member

viccuad commented Nov 14, 2019

Progress in add-yaml-linting branch.

@viccuad
Copy link
Member

viccuad commented Nov 15, 2019

The linting script with yamllint is in place, but I'm having trouble installing yamllint with bazel. I have tried with https://github.com/bazelbuild/rules_python and things in the shape of the following, and a requirements.txt with yamllint, with no luck so far:

(in WORKSPACE)

http_archive(
    name = "rules_python",
    url = "https://github.com/bazelbuild/rules_python/releases/download/0.0.1/rules_python-0.0.1.tar.gz",
    sha256 = "aa96a691d3a8177f3215b14b0edc9641787abaaa30363a080165d06ab65e1161",
)
load('@rules_python//python:pip.bzl', 'pip_import')

pip_import(
    name = "yamllint",
    requirements = "//dev/linters:yamllint_requirements.txt",
)
load("@yamllint//:requirements.bzl", "pip_install")
pip_install()

@viccuad
Copy link
Member

viccuad commented Nov 15, 2019

Right now the viccuad/add-yaml-linting branch contains a solution that:

Adds bazel target //dev/linters:yamllint to install the yamllint python module.
Provides script dev/linters/yamllint.sh that lints all .{yaml,yml} files in the project, analogous to dev/linters/shellcheck.sh.
Updates gitlab-ci to use the new linter.

Bazel's pip_install() is refusing to install pip, so for testing I have a workaround commit that installs pip as a system package in the gitlab image.

To test:

git checkout viccuad/add-yaml-linting
gitlab-runner exec docker lint

TODO:
Find better workaround
Update doc/Contribute.md, doc/linters.md
Fix yaml errors detected by the linter

@viccuad
Copy link
Member

viccuad commented Nov 19, 2019

One implementation of yaml linting (sans helm templates) at #161

@viccuad viccuad assigned viccuad and unassigned viccuad Nov 27, 2019
@mook-as mook-as mentioned this issue Dec 2, 2019
7 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Priority: Medium SUSE SUSE is pursuing a solution Type: CI Issues for working on the project's CI pipelines
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants