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: run actionlint #740

Merged
merged 4 commits into from
Apr 25, 2024
Merged

CI: run actionlint #740

merged 4 commits into from
Apr 25, 2024

Conversation

smelc
Copy link
Contributor

@smelc smelc commented Apr 25, 2024

Changelog

- description: |
    Check workflow files with actionlint
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - improvement    # QoL changes e.g. refactoring
  # - bugfix         # fixes a defect
  - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

actionlint is the defacto checker for GHA workflow files. In my experience, it is quite good and catches a number of mistakes.

How to trust this PR

image

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • Self-reviewed the diff

@smelc smelc force-pushed the smelc/ci-actionlint branch 2 times, most recently from e92184b to d7ee1fb Compare April 25, 2024 10:17
@smelc smelc force-pushed the smelc/ci-actionlint branch from d7ee1fb to 7c74ffd Compare April 25, 2024 10:26
@@ -124,10 +124,6 @@ jobs:
- name: Build Haddock documentation 🔧
run: ./.github/bin/haddocks.sh ./haddocks true

- name: View HTML files
run: |
find "dist-newstyle/build" -name '*.html' | xargs grep html
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This had no effect besides outputting a big number of lines to the pipeline's log.

@smelc smelc marked this pull request as ready for review April 25, 2024 10:27
@smelc smelc requested review from a team as code owners April 25, 2024 10:27
@@ -93,9 +93,9 @@
({pkgs, ...}: {
packages.cardano-cli.configureFlags = [ "--ghc-option=-Werror" ] ++ gitRevFlag;
packages.cardano-cli.components.tests.cardano-cli-test.build-tools =
with pkgs.buildPackages; [ jq coreutils shellcheck ];
with pkgs.buildPackages; [ jq coreutils ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need jq for cli tests? 🤔 I have also doubts about coreutils there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into this next 👍

@@ -0,0 +1 @@
.github/workflows/check-stylish-haskell.yml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with this workflow now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carbolymer> there a number of missing quotes I believe:

.github/workflows/check-stylish-haskell.yml:40:7: shellcheck reported issue in this script: SC2086:info:7:66: Double quote to prevent globbing and word splitting [shellcheck]
   |
40 |       run: |
   |       ^~~~
.github/workflows/check-stylish-haskell.yml:52:7: shellcheck reported issue in this script: SC2086:info:6:22: Double quote to prevent globbing and word splitting [shellcheck]
   |
52 |       run: |
   |       ^~~~
.github/workflows/check-stylish-haskell.yml:52:7: shellcheck reported issue in this script: SC2086:info:9:26: Double quote to prevent globbing and word splitting [shellcheck]
   |
52 |       run: |
   |       ^~~~
.github/workflows/check-stylish-haskell.yml:69:7: shellcheck reported issue in this script: SC2086:info:6:22: Double quote to prevent globbing and word splitting [shellcheck]
   |
69 |       run: |
   |       ^~~~
.github/workflows/check-stylish-haskell.yml:69:7: shellcheck reported issue in this script: SC2086:info:9:26: Double quote to prevent globbing and word splitting [shellcheck]
   |
69 |       run: |
   |       ^~~~

I didn't fix them in this PR, because in my experience, adding checks and fixing warnings in the same PR tend to make the PR harder to merge.

I can make fix them afterwards 👍

@smelc smelc added this pull request to the merge queue Apr 25, 2024
Merged via the queue into main with commit 7b247b3 Apr 25, 2024
22 checks passed
@smelc smelc deleted the smelc/ci-actionlint branch April 25, 2024 12:48
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

Successfully merging this pull request may close these issues.

2 participants