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

v1 Rewrite #34

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

v1 Rewrite #34

wants to merge 4 commits into from

Conversation

sheck
Copy link
Member

@sheck sheck commented Sep 19, 2024

In this PR I rewrote the action in TypeScript and swapped Github check runs for workflow command output powered annotations. This is a culmination of a lot of internal discussion, planning, and unexpected delays.

The core motivation is this: there have been longstanding issues with Github check runs initiated from Github actions, specifically that they get associated with the incorrect action run. Github now recommends using workflow output command annotations for situations like this.

There are also other issues we wanted to address:

  • Check run permission headaches
  • Simplifying dependency install
  • Lack of tests
  • Input and output variable naming using camelCase instead of kebab-case

Breaking changes

  • Similar to when we changed balto-rubocop to use workflow command output, there is no way to have balto-eslint display a neutral conclusion.
    • This has been heavily requested for many years now, but we're still waiting on Github
    • Users can specify a conclusion-level of success (the default) or failure to control how this affects the status of Github checks in the UI.
  • Inputs and outputs use kebab-case now, which matches what Github (eventually) seemed to coalesce on as the standard
  • We no longer manage installing dependencies and instead rely on balto-utils for a cached, speedy install of all JS dependencies
    • There was a non-insignificant amount of complexity involved in maintaining this feature that was implemented before caching was even possible with Github actions

Non-breaking changes

  • We have official support for ESLint v9 now!
  • We're using devbox to ensure we have a consistent development environment
  • Using act for some somewhat manual, somewhat automatic testing
  • I moved the CONTRIBUTING.md file to a section in the README.md

Notes for reviewing

There's a lot here! Sorry! Everything outside of src/ and action.yml are just supporting files though.


This should resolve #3 and #5

@sheck sheck requested a review from a team as a code owner September 19, 2024 20:22
@sheck sheck changed the title Ns/v1 v1 Rewrite Sep 19, 2024
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.

Errors found without any errors on changed files
1 participant