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.

Sign up for GitHub

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

Jump to bottom

ci: update main workflow #139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ci: update main workflow #139

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ on:
- test/**
- bindings/**
- binding.gyp
- .github/workflows/*
pull_request:
branches: [master]
paths:
Expand All @@ -17,6 +18,7 @@ on:
- test/**
- bindings/**
- binding.gyp
- .github/workflows/*
Copy link
Member

Choose a reason for hiding this comment

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

Tbh kinda wonder how worth using these paths are since the jobs are so fast anyway.


concurrency:
group: ${{github.workflow}}-${{github.ref}}
Expand All @@ -29,60 +31,58 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-14]
os: [ubuntu-latest, windows-latest, macos-latest]
steps:
- name: Set up repository
uses: tree-sitter/[email protected]
with:
node-version: 20
- name: Clone repository
Copy link
Member

Choose a reason for hiding this comment

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

nitpick/personal preference/not a blocker: I personally tend to dislike adding name to common actions because it's usually self-explanatory what the action does. It gives off

for i in range(4) # i is an iterator

comment vibes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes the job logs consistent.

uses: actions/checkout@v4
- name: Clone nvim help files
uses: actions/checkout@v4
with:
repository: neovim/neovim
path: examples/neovim
sparse-checkout: runtime/doc/
- name: Run tests
- name: Set up tree-sitter
uses: tree-sitter/setup-action/cli@v1
- name: Run parser tests
uses: tree-sitter/parser-test-action@v2
with:
test-library: ${{runner.os == 'Linux'}}
corpus-files: |-
test-rust: ${{runner.os == 'Linux'}}
Copy link
Member

@dundargoc dundargoc Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't get this. Why only linux? Is it to save CI time? Is it because it's safe to assume that if it works for one system it will work for all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. I don't mind setting it to either false or true though.

- name: Parse sample files
uses: tree-sitter/parse-action@v4
if: runner.os != 'Windows'
Copy link
Member

@dundargoc dundargoc Oct 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Unsure if this or something equivalent was here before the PR, but this seems weird to me. If some of the files can't correctly be parsed on windows (but can on linux), then to me that still indicates that there is a problem we shouldn't ignore. Right now we cannot prevent regressions on windows since we don't test it at all.

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 could make it continue-on-error on Windows so you can still check the summary for failures.

Copy link
Member

Choose a reason for hiding this comment

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

That still won't solve the problem of regressions, and would also condition people into accepting CI failures so I don't think that's optimal. I still don't understand the benefit of ignoring the failures on windows if I'm being honest.

with:
files: |-
examples/neovim/runtime/doc/*
# FIXME: these files should not have errors
!examples/neovim/runtime/doc/remote_plugin.txt
# FIXME: these should not have errors
invalid-files: |-
examples/neovim/runtime/doc/builtin.txt
examples/neovim/runtime/doc/change.txt
examples/neovim/runtime/doc/cmdline.txt
examples/neovim/runtime/doc/dev_style.txt
examples/neovim/runtime/doc/dev_tools.txt
examples/neovim/runtime/doc/develop.txt
examples/neovim/runtime/doc/diagnostic.txt
examples/neovim/runtime/doc/editing.txt
examples/neovim/runtime/doc/eval.txt
examples/neovim/runtime/doc/faq.txt
examples/neovim/runtime/doc/fold.txt
examples/neovim/runtime/doc/ft_ada.txt
examples/neovim/runtime/doc/ft_hare.txt
examples/neovim/runtime/doc/ft_ps1.txt
examples/neovim/runtime/doc/ft_sql.txt
examples/neovim/runtime/doc/help.txt
examples/neovim/runtime/doc/helphelp.txt
examples/neovim/runtime/doc/if_perl.txt
examples/neovim/runtime/doc/if_pyth.txt
examples/neovim/runtime/doc/if_ruby.txt
examples/neovim/runtime/doc/indent.txt
examples/neovim/runtime/doc/index.txt
examples/neovim/runtime/doc/intro.txt
examples/neovim/runtime/doc/job_control.txt
examples/neovim/runtime/doc/lsp.txt
examples/neovim/runtime/doc/luaref.txt
examples/neovim/runtime/doc/map.txt
examples/neovim/runtime/doc/mbyte.txt
examples/neovim/runtime/doc/motion.txt
examples/neovim/runtime/doc/news.txt
examples/neovim/runtime/doc/nvim.txt
examples/neovim/runtime/doc/options.txt
examples/neovim/runtime/doc/pattern.txt
examples/neovim/runtime/doc/pi_gzip.txt
examples/neovim/runtime/doc/pi_health.txt
examples/neovim/runtime/doc/pi_msgpack.txt
examples/neovim/runtime/doc/pi_netrw.txt
examples/neovim/runtime/doc/pi_paren.txt
Expand All @@ -92,7 +92,6 @@ jobs:
examples/neovim/runtime/doc/provider.txt
examples/neovim/runtime/doc/quickfix.txt
examples/neovim/runtime/doc/quickref.txt
examples/neovim/runtime/doc/remote_plugin.txt
examples/neovim/runtime/doc/repeat.txt
examples/neovim/runtime/doc/russian.txt
examples/neovim/runtime/doc/starting.txt
Expand All @@ -107,7 +106,6 @@ jobs:
examples/neovim/runtime/doc/usr_12.txt
examples/neovim/runtime/doc/usr_22.txt
examples/neovim/runtime/doc/usr_28.txt
examples/neovim/runtime/doc/usr_29.txt
examples/neovim/runtime/doc/usr_41.txt
examples/neovim/runtime/doc/various.txt
examples/neovim/runtime/doc/vim_diff.txt
Expand Down