Skip to content

feat: Add lua-language-server to CI #2611

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

Merged
merged 9 commits into from
Aug 1, 2024

Conversation

fidgetingbits
Copy link
Collaborator

  • Implement lua-language-server linter in pre-commit
  • Install via Nix due to lack of Ubuntu package
  • Apply patch for --check CLI bug using Nix overlay
  • Centralize linting config for VSCode, pre-commit, and CI

Fixes #2576

Checklist

- Implement lua-language-server linter in pre-commit
- Install via Nix due to lack of Ubuntu package
- Apply patch for --check CLI bug using Nix overlay
- Centralize linting config for VSCode, pre-commit, and CI

Fixes cursorless-dev#2576
@fidgetingbits fidgetingbits added the app-neovim Issues related to neovim support label Jul 31, 2024
@fidgetingbits fidgetingbits requested review from saidelike and pokey July 31, 2024 10:45
@fidgetingbits fidgetingbits requested a review from a team as a code owner July 31, 2024 10:45
@pokey
Copy link
Member

pokey commented Jul 31, 2024

I wonder if we want to do this in pre-commit or as part of test. This feels like running tsc, which we do as part of our test CI step. I'm not in love with needing nix set up in order for pre-commit to work locally, so doing it as part of test makes it easier to just be CI-only / manually triggered locally

@fidgetingbits
Copy link
Collaborator Author

Ok. I moved it into test.yml. I went the route of having another combined action again so we don't have to litter multiple entries with 'if linux'. Didn't manually test these new changes yet, so will see how the CI goes.

Copy link
Member

@pokey pokey left a comment

Choose a reason for hiding this comment

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

Looks good with minor comments. Feel free to merge once addressed. I dig that sub-action thing. We could do more of that

@fidgetingbits fidgetingbits enabled auto-merge July 31, 2024 14:15
@fidgetingbits fidgetingbits disabled auto-merge July 31, 2024 14:18
@fidgetingbits
Copy link
Collaborator Author

Any idea why the 'ubuntu-latest, stable' CI test would've skipped here? That's the only one that will run the new lua test and was running before, so seems a bit weird... but details don't show anything. I don't seem to be able to force re-run it

@pokey
Copy link
Member

pokey commented Jul 31, 2024

Hmm strange. Maybe try pushing an empty commit?

@fidgetingbits fidgetingbits changed the title feat: Add lua-language-server to pre-commit hooks feat: Add lua-language-server to CI Aug 1, 2024
@fidgetingbits fidgetingbits added this pull request to the merge queue Aug 1, 2024
Merged via the queue into cursorless-dev:main with commit 6d6addc Aug 1, 2024
15 checks passed
@fidgetingbits fidgetingbits deleted the lua-ls-linting branch August 1, 2024 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app-neovim Issues related to neovim support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

neovim: add lua language server lint checks to CI
2 participants