Absorb `actionlint` into pre-commit #1028

AdarshRawat1 · 2023-03-25T10:25:18Z

Fixes

Description

The current setup of running 'actionlint' as a separate workflow on push and pull requests can be problematic as it only reports issues after they have been pushed, which can be time-consuming and potentially cause problems for the development process.

This PR integrates actionlint into the pre-commit process.

Effects -

developers can run checks locally before committing and pushing, allowing them to catch any issues and fix them before they get pushed to the main branch.
eliminates the need for a separate workflow and instead incorporates the check as part of the Lint job inside the CI + CD workflow.

Checklist

My pull request has a descriptive title (not a vague title like
Update index.md).
My pull request targets the default branch of the repository (main) or
a parent feature branch.
My commit messages follow best practices.
My code follows the established code style of the repository.
I added or updated tests for the changes I made (if applicable).
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no visible
errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

- added actionlint to .pre-commit-config.yaml file - deleted the actionlint.yml file from .github/workflows.

Added '- actionlint' to hooks in pre-commit-config template of `.pre-commit-config.yaml` file.

dhruvkb

@AdarshRawat1 thanks for this PR, it is in the right direction but I do have a few changes for you.

templates/.pre-commit-config.yaml.jinja

Co-authored-by: Dhruv Bhanushali <dhruv_b@live.com>

dhruvkb

Awesome, only one more step left to go!

.pre-commit-config.yaml

Incorrect review

dhruvkb · 2023-03-31T11:03:21Z

Weirdly actionlint is reporting our custom actions from .github/actions as incorrect. Actions have a different syntax compared to workflows and I expected actionlint to be aware of that. Maybe someone from @WordPress/openverse-maintainers might have any idea.

sarayourfriend · 2023-04-01T23:26:33Z

Actionlint can't lint actions, only workflows (TIL): rhysd/actionlint#46

I think the issue is the files directive. The default files directive for actionlint already scopes it to .github/workflows. Adding the actions directory as well includes files that actionlint doesn't know what to do with. I think removing the files directive altogether is fine and just relying on the default: https://github.com/rhysd/actionlint/blob/main/.pre-commit-hooks.yaml#L14

dhruvkb · 2023-04-02T08:46:30Z

Actionlint can't lint actions, only workflows (TIL): rhysd/actionlint#46

Wow. Pretty misleading name then!

I agree with @sarayourfriend regarding the fix.

sarayourfriend

LGMT. @dhruvkb I'll open a PR to remove the actionlint required check from the repo configuration now that it's folded into pre-commit.

dhruvkb

This is nice.

sarayourfriend · 2023-04-02T23:33:45Z

Rebasing to get the checks to re-run and update with the modified required checks list which now excludes the no-longer existent actionlint workflow.

* Fix issues in the workflow simplifications of #1054 (#1058) * Retry `up` recipe in case port is occupied (#990) * Fix typo in docs building on `main` (#1067) * Restore Django Admin views (#1065) * Update other references of media count to 700 million (#1098) * Dispatch workflows instead of regular reuse to show deployment runs (#1034) * Use label.yml to determine required labels (#1063) Co-authored-by: Dhruv Bhanushali <hi@dhruvkb.dev> * Add `GITHUB_TOKEN` to GitHub CLI step (#1103) * Pass actor for staging deploys with the `-f` flag (#1104) * Bump ipython from 8.11.0 to 8.12.0 in /api (#1113) Bumps [ipython](https://github.com/ipython/ipython) from 8.11.0 to 8.12.0. - [Release notes](https://github.com/ipython/ipython/releases) - [Commits](ipython/ipython@8.11.0...8.12.0) --- updated-dependencies: - dependency-name: ipython dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Absorb `actionlint` into pre-commit (#1028) Co-authored-by: Dhruv Bhanushali <dhruv_b@live.com> Co-authored-by: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com> * Bump orjson from 3.8.8 to 3.8.9 in /api (#1114) Bumps [orjson](https://github.com/ijl/orjson) from 3.8.8 to 3.8.9. - [Release notes](https://github.com/ijl/orjson/releases) - [Changelog](https://github.com/ijl/orjson/blob/master/CHANGELOG.md) - [Commits](ijl/orjson@3.8.8...3.8.9) --- updated-dependencies: - dependency-name: orjson dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add Sentry to the ingestion server (#1106) * Add a wait to filter button test to fix CI (#1124) * Bump boto3 from 1.26.100 to 1.26.104 in /ingestion_server (#1110) * Bump sentry-sdk from 1.17.0 to 1.18.0 in /api (#1112) * Bump pillow from 9.4.0 to 9.5.0 in /api (#1115) * Update redis Docker tag to v4.0.14 (#1109) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * 🔄 synced file(s) with WordPress/openverse-infrastructure (#1127) Co-authored-by: openverse-bot <null> * Update other references of media count to 700 million (#1100) * Fix prod deployment workflow dispatch call (#1117) * Add a Slack notification job to the CI + CD workflow (#1066) * Fix types in VFilters and VContentReport (#1030) * Move the svgs for radiomark and check to components * Add files to tsconfig and fix types * Mock report service in the unit test * Type svg?inline as vue Component * Better License code type checking * Update frontend/src/components/VFilters/VFilterChecklist.vue * Revert unnecessary changes * Update frontend/src/components/VFilters/VFilterChecklist.vue Co-authored-by: Zack Krida <zackkrida@pm.me> * Rename `ownValue` to `value_` --------- Co-authored-by: Zack Krida <zackkrida@pm.me> * Convert VPill and VItemGroup stories to mdx (#1092) * Convert VPill story to MDX * Convert VItemGroup story to mdx * Fixing argTypes issue and fixing the headers * Update ci to use github.token (#1123) * Add `SLACK_WEBHOOK_TYPE` env var to reporting job (#1131) * Add consent decision-making process documentation (#887) * Prepare VButton for updates (#1002) * Rename button sizes and apply some styles only to 'old' buttons * Rename the snapshot tests to v-button-old * Fix VTab focus style * Small fixes (large-old, border, group/button) * Revert VTab focus changes Moved to a different PR * Revert "Revert VTab focus changes" This reverts commit ec9312d. * Use only focus-visible for consistency * Bump boto3 from 1.26.99 to 1.26.105 in /api (#1133) Bumps [boto3](https://github.com/boto/boto3) from 1.26.99 to 1.26.105. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](boto/boto3@1.26.99...1.26.105) --- updated-dependencies: - dependency-name: boto3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add more docs for Plausible and auto-initialise custom event names (#1122) * Add more docs for Plausible and auto-initialise custom event names * Update existing docs * Add caveat that it is not necessary to run Plausible if not working on custom events * Fix ToC * Add new buttons variants and sizes (#1003) * Add new VButton sizes and variants * Add new Storybook tests * Add border to transparent- buttons * Update bordered and transparent buttons * Update stories * Update snapshots * Remove pressed variants * Add missing snapshots * Fix transparent buttons * Update paddings In accordance with #860 (comment) * Update snapshots * Update frontend/src/components/VButton.vue Co-authored-by: Zack Krida <zackkrida@pm.me> --------- Co-authored-by: Zack Krida <zackkrida@pm.me> * Pass `GITHUB_TOKEN` to deploy docs (#1134) * Add context manager and join() --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Dhruv Bhanushali <hi@dhruvkb.dev> Co-authored-by: Krystle Salazar <krystle.salazar@automattic.com> Co-authored-by: Madison Swain-Bowden <bowdenm@spu.edu> Co-authored-by: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com> Co-authored-by: Tomvth <50104959+Tomvth@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Adarsh Rawat <adarshrawat141@gmail.com> Co-authored-by: Dhruv Bhanushali <dhruv_b@live.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Openverse (Bot) <101814513+openverse-bot@users.noreply.github.com> Co-authored-by: Zack Krida <zackkrida@pm.me> Co-authored-by: Sepehr Rezaei <54465154+sepehrrezaei@users.noreply.github.com> Co-authored-by: Sumit Kashyap <mr.sumitkrr@gmail.com>

* Improve license URL validation * Use the actual URL instead of the part * Fix incorrect jamendo test * Add tests _get_license_url adds a trailing slash and does not make an internet request for known licenses Signed-off-by: Olga Bulat <obulat@gmail.com>

AdarshRawat1 and others added 2 commits March 25, 2023 13:36

Absorbed actionlint into pre-commit

297e87b

- added actionlint to .pre-commit-config.yaml file - deleted the actionlint.yml file from .github/workflows.

Added 'actionlint' to pre-commit-config template

21f29fc

Added '- actionlint' to hooks in pre-commit-config template of `.pre-commit-config.yaml` file.

AdarshRawat1 requested a review from a team as a code owner March 25, 2023 10:25

AdarshRawat1 requested review from AetherUnbound and stacimc March 25, 2023 10:25

openverse-bot added 🟩 priority: low Low priority and doesn't need to be rushed 🌟 goal: addition Addition of new feature 🤖 aspect: dx Concerns developers' experience with the codebase 🧱 stack: mgmt Related to repo management and automations labels Mar 25, 2023

AdarshRawat1 added 2 commits March 25, 2023 17:00

Added actionlint to pre-commit-config

5ab00ab

removed extra whitespaces

0fdf652

dhruvkb requested changes Mar 27, 2023

View reviewed changes

templates/.pre-commit-config.yaml.jinja Outdated Show resolved Hide resolved

AdarshRawat1 and others added 2 commits March 31, 2023 07:47

Update templates/.pre-commit-config.yaml.jinja

569bdff

Co-authored-by: Dhruv Bhanushali <dhruv_b@live.com>

Merge branch 'main' into Absorb-actionlint

3c4713b

dhruvkb previously approved these changes Mar 31, 2023

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

Update .pre-commit-config.yaml

ad750b7

AdarshRawat1 and others added 2 commits April 2, 2023 21:24

Removed the files directive for actionlint-docker.

38680af

Fix template to pass lint checks

ee26867

sarayourfriend approved these changes Apr 2, 2023

View reviewed changes

dhruvkb approved these changes Apr 2, 2023

View reviewed changes

Merge branch 'main' into Absorb-actionlint

756e937

dhruvkb merged commit 10d757e into WordPress:main Apr 2, 2023

AdarshRawat1 deleted the Absorb-actionlint branch April 3, 2023 03:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Absorb `actionlint` into pre-commit #1028

Absorb `actionlint` into pre-commit #1028

AdarshRawat1 commented Mar 25, 2023

dhruvkb left a comment

dhruvkb left a comment

dhruvkb commented Mar 31, 2023

sarayourfriend commented Apr 1, 2023 •

edited

Loading

dhruvkb commented Apr 2, 2023 •

edited

Loading

sarayourfriend left a comment

dhruvkb left a comment

sarayourfriend commented Apr 2, 2023

Absorb actionlint into pre-commit #1028

Absorb actionlint into pre-commit #1028

Conversation

AdarshRawat1 commented Mar 25, 2023

Fixes

Description

Checklist

Developer Certificate of Origin

dhruvkb left a comment

Choose a reason for hiding this comment

dhruvkb left a comment

Choose a reason for hiding this comment

dhruvkb commented Mar 31, 2023

sarayourfriend commented Apr 1, 2023 • edited Loading

dhruvkb commented Apr 2, 2023 • edited Loading

sarayourfriend left a comment

Choose a reason for hiding this comment

dhruvkb left a comment

Choose a reason for hiding this comment

sarayourfriend commented Apr 2, 2023

Absorb `actionlint` into pre-commit #1028

Absorb `actionlint` into pre-commit #1028

sarayourfriend commented Apr 1, 2023 •

edited

Loading

dhruvkb commented Apr 2, 2023 •

edited

Loading