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

[WIP]: add custom groups #1799

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ykanavalik
Copy link

@ykanavalik ykanavalik commented Jun 3, 2020

[DARFT]

Based on issue: #1015
Adds a possibility to pass custom glob pattern as a custom group

Usage example

"groups": [
	"builtin",
	"external",
	"consts/**/*",
	"utils/**/*",
	"sibling",
]

@ykanavalik
Copy link
Author

guys) would be really nice to have such feature
let's discuss better API if that one is not okay

@coveralls
Copy link

Coverage Status

Coverage decreased (-5.0%) to 92.784% when pulling d85e5de on ZennKa:feature/add-custom-groups into e1ed323 on benmosher:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-5.0%) to 92.784% when pulling d85e5de on ZennKa:feature/add-custom-groups into e1ed323 on benmosher:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-5.0%) to 92.784% when pulling d85e5de on ZennKa:feature/add-custom-groups into e1ed323 on benmosher:master.

@ykanavalik
Copy link
Author

@ljharb that's my first PR into this repo
suppose, i can ping you :)

what do you think if such api?

i'm trying to understand if such solution is okay and continue development if so

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks for the ping :-) I think this is a reasonable direction; i'd hope for a few more test cases, and especially some that test that gitignored/eslintignored files are not included even if a glob covers them.

Comment on lines +86 to +90
for (let globPatter of groups) {
if (minimatch(name, globPatter)) {
return globPatter
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (let globPatter of groups) {
if (minimatch(name, globPatter)) {
return globPatter
}
}
return groups.find(globPattern => minimatch(name, globPattern)

@tomasloksa
Copy link

Hey, is this being worked on? I could really use this feature. I could even help with it - just let me know what else needs to be done.

@ljharb
Copy link
Member

ljharb commented Oct 2, 2020

@tomasloksa if you'd like to help, please feel free to comment with a link to your own branch, and i'll pull the commits into this PR :-)

@ykanavalik ykanavalik closed this Jul 10, 2022
@ljharb ljharb reopened this Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants