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

Generate CI filter for files included in oxc_ast_codegen #4699

Closed
rzvxa opened this issue Aug 6, 2024 · 5 comments · Fixed by #4737
Closed

Generate CI filter for files included in oxc_ast_codegen #4699

rzvxa opened this issue Aug 6, 2024 · 5 comments · Fixed by #4737
Labels
C-enhancement Category - New feature or request

Comments

@rzvxa
Copy link
Collaborator

rzvxa commented Aug 6, 2024

Generate what files are watched, It is currently manually written here:

filters: |
src:
- 'crates/oxc_ast/src/ast/*' # single star is intentional
- 'crates/oxc_ast/src/generated/**' # to potentially cause CI error if generated files are edited manually
- 'crates/oxc_syntax/src/number.rs'
- 'crates/oxc_syntax/src/operator.rs'
- 'tasks/ast_codegen/src/**'

@rzvxa rzvxa added C-bug Category - Bug C-enhancement Category - New feature or request and removed C-bug Category - Bug labels Aug 6, 2024
@overlookmotel
Copy link
Collaborator

overlookmotel commented Aug 6, 2024

Can we just simplify it to:

filters: | 
  src: 
    - 'crates/oxc_ast/src/**'
    - 'crates/oxc_syntax/src/**'
    - 'crates/oxc_span/src/**'
    - 'tasks/ast_codegen/src/**' 

I know that casts the net a bit wider than it needs to, but:

  1. It's foolproof.
  2. These crates don't get that much churn (compared to e.g. oxc_linter, oxc_transformer), so job still wouldn't run on 95% of PRs.
  3. Somehow generating a ci.yml file feels like overkill.

@rzvxa
Copy link
Collaborator Author

rzvxa commented Aug 6, 2024

If I'm not mistaken filter can be its own file, So we only generate a simple .github/ast_codegen_filter.yml.
https://github.com/dorny/paths-filter?tab=readme-ov-file#advanced-options
It shouldn't be anything above a few lines of generator code(we just concat string instead of serializing an actual yaml).

A simple generator not only makes sure that all of our current paths are being watched but it would also add new paths to the list as we extend upon it. That's why I felt it would be a good way to set it up and just forget about it.

@overlookmotel
Copy link
Collaborator

Ah OK. If it's that easy then, yes, seems like a good idea.

@rzvxa
Copy link
Collaborator Author

rzvxa commented Aug 6, 2024

Ah OK. If it's that easy then, yes, seems like a good idea.

I'm optimistic that it is quite trivial, I hope time doesn't prove me wrong😆

overlookmotel pushed a commit that referenced this issue Aug 7, 2024
closes #4699

I also did some reordering to the code so it is easier to follow.
@overlookmotel
Copy link
Collaborator

Implemented in #4737.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category - New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants