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

exclude-file-types flag exludes aliases #216

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

veirfuna
Copy link
Contributor

@veirfuna veirfuna commented Nov 9, 2024

fix for #214

If file type is similar to one of extensions, then exclude all extensions

Before fix:
Screenshot 2024-11-09 at 19 46 30

After fix:
Screenshot 2024-11-09 at 19 48 17

Copy link
Contributor

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

I don't think it implements everything I raised here.

#214 (comment)

But it's a good start for an implementation

cmd/validator/validator.go Show resolved Hide resolved
name: "exclude json and yaml",
input: "json,yaml",
expectedExcludeFileTypes: []string{"json", "yaml", "yml"},
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add tests about these cases I reported here

#216 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added test

@ccoVeille
Copy link
Contributor

By aliases, I meant something like the aliases you can find here

https://github.com/github-linguist/linguist/blob/main/lib%2Flinguist%2Flanguages.yml

I don't expect to implement everything of course.

And yes, with the current file types, your implementation could be enough

https://github.com/Boeing/config-file-validator/blob/main/pkg%2Ffiletype%2Ffile_type.go

The only examples in the current list, I can think about is the .env, that people are more likely to call dotenv, so it could be an alias

@kehoecj kehoecj self-requested a review November 11, 2024 14:15
@kehoecj kehoecj added OSS Community Contribution Contributions from the OSS Community waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers labels Nov 11, 2024
Copy link
Collaborator

@kehoecj kehoecj left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @veirfuna for the PR!

@kehoecj kehoecj merged commit 19ca4d0 into Boeing:main Dec 17, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OSS Community Contribution Contributions from the OSS Community waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants