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

Use glob_match instead of globset #24

Open
zifeo opened this issue Jan 17, 2023 · 2 comments · May be fixed by #99
Open

Use glob_match instead of globset #24

zifeo opened this issue Jan 17, 2023 · 2 comments · May be fixed by #99

Comments

@zifeo
Copy link
Owner

zifeo commented Jan 17, 2023

https://github.com/devongovett/glob-match

@zifeo zifeo changed the title User glob_match instead of globset Use glob_match instead of globset Jan 17, 2023
@abdiu34567
Copy link

Hello! 👋

I'm interested in working on this issue to replace globset with glob-match. I've taken a preliminary look at the codebase, and I believe I can tackle this. Before I dive in, I'd like to clarify a few points:

Reason for Switch: Can you provide a bit more context about why we're considering this switch? Are there any specific advantages of glob-match over globset that I should be aware of?

Backward Compatibility: Do we aim to ensure complete backward compatibility in behavior, or are slight deviations acceptable as long as the primary functionality remains consistent?

Existing Tests: Are there existing tests that cover the functionalities using globset? If yes, I'll ensure they all pass after the switch. If not, I'll aim to write tests to ensure the behavior is consistent.

Deprecation: Post the switch, do we intend to deprecate all globset functionalities immediately or will there be a transition phase?

Looking forward to your insights. Once I have a clearer picture, I'll start with the implementation.

@zifeo
Copy link
Owner Author

zifeo commented Oct 3, 2023

@abdiu34567 Thanks for asking for some details:

  • Reason for Switch: glob-match provides more advanced syntax, and globset is primarily used for ripgred, it might not fit other use cases. Also see the benchmark in the repo, it is significantly faster.
  • Backward Compatibility: deviation acceptable if their improve the user experience.
  • Existing Tests: none I believe. However if you find a way to test this without increasing too much the surface maintenance, welcome to add some.
  • Deprecation: hard switch, no need to keep previous version working.

@zifeo zifeo closed this as completed Oct 3, 2023
@zifeo zifeo reopened this Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants