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

just: match both yaml and yml #1147

Closed
wants to merge 1 commit into from
Closed

Conversation

msanft
Copy link
Contributor

@msanft msanft commented Jan 14, 2025

This fixes a little papercut I ran into when working with the .custom directory, where I wondered why my files weren't included. Turned out we only match against .yml while I used .yaml. This makes it so both endings are valid and included.

@msanft msanft added the no changelog PRs not listed in the release notes label Jan 14, 2025
@msanft msanft added this to the v1.4.0 milestone Jan 14, 2025
@msanft msanft requested a review from katexochen January 14, 2025 07:36
@katexochen katexochen removed this from the v1.4.0 milestone Jan 14, 2025
@msanft msanft requested a review from burgerdev January 20, 2025 12:24
Copy link
Contributor

@burgerdev burgerdev left a comment

Choose a reason for hiding this comment

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

Wait, doesn't this result in a glob failure if there's either no yml or no yaml?

@msanft
Copy link
Contributor Author

msanft commented Jan 20, 2025

Wait, doesn't this result in a glob failure if there's either no yml or no yaml?

@burgerdev no, you can test this by:

$ ll *.{yml,yaml}
"*.yml": No such file or directory (os error 2)
"*.yaml": No such file or directory (os error 2)
$ ll *.yml
"*.yml": No such file or directory (os error 2)

@msanft msanft requested a review from burgerdev January 20, 2025 12:56
@burgerdev
Copy link
Contributor

That fails for me:

$ touch a.yaml
$ ls *.{yml,yaml}
ls: cannot access '*.yml': No such file or directory
 a.yaml
$ echo $?
2

@msanft
Copy link
Contributor Author

msanft commented Jan 20, 2025

Oh, i missed the either. Do you have a suggestion on how to glob this instead? *.y{a,}ml? That would probably all result in the same thing?

@burgerdev
Copy link
Contributor

We could set shopt -s nullglob (docs).

This fixes a little papercut I ran into when working with the `.custom`
directory, where I wondered why my files weren't included. Turned out we
only match against `.yml` while I used `.yaml`. This makes it so both
endings are valid and included.
@msanft msanft force-pushed the msanft/just/match-yaml-yml branch from 0d210ec to 3a219cf Compare January 20, 2025 13:24
@msanft
Copy link
Contributor Author

msanft commented Jan 20, 2025

Tested this with yml files only, and with both yml and yaml.

@katexochen
Copy link
Member

Please don't overengineer this. We use .yml by convention.

@msanft msanft closed this Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog PRs not listed in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants