Skip to content

Linting: Vint

Matthew Armand requested to merge matthewarmand/pacman-contrib:lint-vint into master

Aim

This MR accomplishes part of #11, namely to lint the Vim files in the package with vint. This is partially an MR of the vint pipeline step directly, but should also serve as a vetting of the standards set by the vint job, which will surely be mimicked by other linting jobs that come along next. Vint is a good one to start with because it:

  1. Is in the official repos
  2. Has very little feedback for existing code, and so is easy to fix. I'd mostly be comfortable addressing the Shellcheck feedback but there's quite a lot of it, and there's minimal CPPCheck feedback but that's not as much my forte.

Workflow

The workflow section of the pipeline saves us from having to specify the same running rules to all the jobs, Gitlab docs here: https://docs.gitlab.com/ee/ci/yaml/workflow.html

Structure

I added the job without separating them out into stages, which in practice puts the vint and test jobs together in an implicit stage called test, and the jobs can run in parallel. I considered making a separate stage for lint but since there isn't actually a run dependency between the stages, that seemed unnecessary.

Another option is in Merge Requests, running the linter only when files governed by it are touched. This has the advantage that if a linter changes styling, someone contributed unrelated code won't be hit by it; the pipeline would just fail after merge to master. I haven't implemented that here but can if that's desired.

This is what the pipeline jobs look like structurally now then:

image

Vint

I decided to use Arch as the base image because Vint is easily found in the official repos and it mimics the test job. This could have the side effect that if Vint (or another linter) changes its styling, pipelines here will start failing as soon as that update hits Arch, and updates will need to be made to appease the linter. This could be avoided by using a different base image, pinning to a specific Vint version, and updating intentionally over time, but honestly that's more likely to fall out of date so I don't prefer it.

I ended up using bash's globstar shopt as that felt like the simplest way to make this happen in just one invocation of Vint, and therefore just have one exit code to decide the failure state of the pipeline. find, xargs, and other options had these or similar pitfalls.

You can see an example of a failed pipeline here: https://gitlab.archlinux.org/matthewarmand/pacman-contrib/-/pipelines/72749 with its failed vint job here: https://gitlab.archlinux.org/matthewarmand/pacman-contrib/-/jobs/165013

And a successful pipeline here, after addressing the linter's feedback: https://gitlab.archlinux.org/matthewarmand/pacman-contrib/-/pipelines/72751 with its successful vint job here: https://gitlab.archlinux.org/matthewarmand/pacman-contrib/-/jobs/165019

Explanation of linter feedback changes here: https://vimhelp.org/autocmd.txt.html#autocmd-groups

Let me know if you have any questions, or feel anything should be different.

Edited by Matthew Armand

Merge request reports