search

adrienverge / yamllint

A linter for YAML files.
GNU General Public License v3.0
2.83k stars 269 forks source link

Document that GitHub Actions already installs `yamllint` #588

Closed jeffwidman closed 4 months ago

jeffwidman commented 1 year ago

I was looking up instructions for how to install yamllint so that I could use it in our GitHub actions workflow when I happened to see mention that it's already installed in the base image.

So adding a quick note here.

I phrased the note generically because additional CI systems may (should!! 😄 ) start doing this, so we can easily add them to the list.

jeffwidman commented 1 year ago

Oh crazy, I didn't realize this is literally all I need (+ a .yamllint config file for customization) to get this running in Github Actions:

---
name: yamllint
on:  # yamllint disable-line rule:truthy
  pull_request:
    branches: ["main"]

jobs:
  yamllint:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3

      # yamllint is installed in GitHub Actions base runner image: https://github.com/adrienverge/yamllint/pull/588
      - run: yamllint .

We should probably document this better so that folks who want vanilla yamllint aren't first having their github action workflow download the docker image/apt-get install etc which wastes energy/bandwidth.

adrienverge commented 1 year ago

Hello and thanks for the proposal! You're right: yamllint is now installed by default on some platforms. However, I don't see the purpose of such a new note, especially placed before the how-to-install on popular OSes. Having this note looks more confusing to me than not having it, and we're trying to keep instructions short for clarity.

coveralls commented 1 year ago

Coverage Status

coverage: 99.825%. remained the same when pulling 8744b7a7e24574e55eaa3c06248b3e89db2c6c0e on jeffwidman:document-some-CI-systems-already-install-yamllint into 81e9f98ffd059efe8aa9c1b1a42e5cce61b640c6 on adrienverge:master.

jeffwidman commented 1 year ago

Makes sense to me about keeping the user experience short/uncluttered, so any suggestions for how to make users aware that they don't even need to install yamllint in order to run it in GitHub actions?

One of the most common places to install yamllint is in CI systems... I suspect if you could poll users for "why are you looking at the install instructions?" many would say "I'm trying to figure out how to wire it into our CI system"...

Currently I'm setting up yamllint for :dependabot:, and before I worked at GitHub, I setup yamllint multiple times at previous jobs and each time I had to hunt through the instructions trying to figure out how to install yamllint in order to get it to run. I'm sure this happens routinely for other folks as well. Plus again, since this install is happening in CI, that results in needlessly downloading / installing it hundreds of thousands of times a year vs if they knew it was already baked into the golden image it'd save human time, machine time, network bandwidth, and electricity.

If you don't want it as a "note" at the top, would you be open to a PR listing it as a another type of install? I could list it right below OpenBSD.

andrewimeson commented 1 year ago

If you rely on yamllint being implicitly available in your CI environment, contributors will not get the benefits of yamllint locally unless they install it. Even if they do install it they may have a different version that produces different results than what is in CI. It's best to pin dependencies like yamllint either in a requirements.txt or requirements-dev.txt file.

adrienverge commented 1 year ago

If you don't want it as a "note" at the top, would you be open to a PR listing it as a another type of install? I could list it right below OpenBSD.

If this is to be merged, a small sentence at the top looks good to me :+1: (even if it's just a simple sentence, not in a note)

However, I'm not sure such a new paragraph adds clarity:

I'm not opposed to a very simple introduction phrase like this, if you really think it adds value:

yamllint is already installed on some systems (like GitHub Actions base image), but in other cases, here is how to install it on some popular systems.

jeffwidman commented 4 months ago

Even if they do install it they may have a different version that produces different results than what is in CI. It's best to pin dependencies like yamllint either in a requirements.txt or requirements-dev.txt file.

There's a variety of opinions on this. I work on :dependabot:, a product that automates bumping pinned dependencies every day, so I'm well aware of the benefits of pinning. But for dev tooling such as linters, it's quite common for maintainers to opt for the minimal maintenance overhead of "floating" latest, and only if they hit problems will they temp pin.

That's probably because the consequences of dev tooling blowing up is that typically it only kills your CI pipeline. Totally different story from needing to pin your app deps to prevent them blowing up unexpectedly in production.

Not uncommon to see smaller projects do this for pytest, flake8, ubuntu build OS versions, etc.

jeffwidman commented 4 months ago

I updated to a simple sentence.

I did link to the example, as I suspect many folks will be looking for a copy/paste example of how to run it on GitHub Actions.

It's been a bit since I worked with RST, so my apologies if I got the formatting wrong.

Also, given the lukewarm reception to this PR, no problem if you don't want this in your docs. It's really up to you.

adrienverge commented 4 months ago

Hello Jeff, thanks for following up and reviving this! The formatting looks good (doc8 and rstcheck pass :+1:) Sorry about the lukewarm reception. I prefer being honest: I have the feeling that proposed new sentence in the documentation page will help only a few users, while making the documentation longer to read for many.

jeffwidman commented 4 months ago

No problem, I'll close it. I disagree given what I've seen at multiple employers... the conversation typically goes like this:

"oh crap, another yaml bug we missed bit us" "yeah, we should enable yamllint in CI" "yeah, I've been meaning to do that after we got bit by a similar bug last year, we filed a ticket to do it, but no one has ever quite gotten to it..." "did you realize it's only a 1-liner since it's already installed in github actions?" "no, I had no idea!"

Clearly you've had a different experience though.