Add support for double star and glob patterns#145
Add support for double star and glob patterns#145rustatian merged 1 commit intoroadrunner-server:masterfrom
Conversation
📝 WalkthroughWalkthroughThis PR enables advanced glob pattern support for proto file path configuration by replacing the standard filepath.Glob with doublestar.FilepathGlob. This allows recursive globstar (**) and brace expansion ({...}) patterns in proto path specifications. The changes include a new external dependency, updated pattern matching logic, and expanded test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
go.mod (1)
30-30: Promotedoublestar/v4to a direct requirement.
config.goimports this module directly, so keeping it marked// indirectwill be rewritten by the nextgo mod tidy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 30, The go.mod entry for github.com/bmatcuk/doublestar/v4 is marked "// indirect" but config.go imports it directly; update go.mod to make doublestar a direct requirement by removing the "// indirect" marker (or run `go get github.com/bmatcuk/doublestar/v4@v4.10.0`) so the module remains listed as a direct dependency; check config.go to confirm the import usage after the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@parser/test_nested/sub/deep.proto`:
- Line 2: The proto package declaration "package deep;" in the file containing
the fixture under parser/test_nested/sub causes a PACKAGE_DIRECTORY_MATCH lint
failure; fix it by either updating the package declaration to match the
directory (e.g., change the package name from "deep" to the correct package for
parser.test_nested.sub) or move this proto file into a deep/ subdirectory so the
package "deep" and the file path align; locate the package line ("package
deep;") and apply the chosen fix consistently across imports and references.
---
Nitpick comments:
In `@go.mod`:
- Line 30: The go.mod entry for github.com/bmatcuk/doublestar/v4 is marked "//
indirect" but config.go imports it directly; update go.mod to make doublestar a
direct requirement by removing the "// indirect" marker (or run `go get
github.com/bmatcuk/doublestar/v4@v4.10.0`) so the module remains listed as a
direct dependency; check config.go to confirm the import usage after the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4aa000e7-7996-4ebd-a153-977588ead902
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
config.goconfig_test.gogo.modparser/test_nested/sub/deep.proto
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #145 +/- ##
=======================================
Coverage 68.31% 68.31%
=======================================
Files 7 7
Lines 464 464
=======================================
Hits 317 317
Misses 113 113
Partials 34 34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
lgtm, thank you @Nyholm 👍🏻 |
There was a problem hiding this comment.
Pull request overview
This PR extends the grpc.proto configuration handling to support more expressive globbing—specifically recursive ** patterns and brace expansion—so users can specify proto file sets via glob patterns (fixing roadrunner-server/roadrunner#2220 and building on PR #90).
Changes:
- Switch proto path expansion from
filepath.Globtodoublestar.FilepathGlobto support**and{...}patterns. - Add unit tests covering recursive
**and brace-expansion cases. - Add nested proto fixtures to validate recursive matching.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
config.go |
Uses doublestar.FilepathGlob and treats { as a glob indicator when expanding Config.Proto. |
config_test.go |
Adds test coverage for **/*.proto recursion and {a,b} / {pattern1,pattern2} brace expansion. |
parser/test_nested/sub/deep.proto |
Adds a deeper nested proto fixture for recursive glob tests. |
go.mod |
Adds github.com/bmatcuk/doublestar/v4 (currently placed in the indirect section). |
go.sum |
Adds checksums for github.com/bmatcuk/doublestar/v4. |
go.work.sum |
Updates workspace checksums (likely from dependency resolution/build/test). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| github.com/beorn7/perks v1.0.1 // indirect | ||
| github.com/bmatcuk/doublestar/v4 v4.10.0 // indirect | ||
| github.com/cespare/xxhash/v2 v2.3.0 // indirect |
There was a problem hiding this comment.
github.com/bmatcuk/doublestar/v4 is imported from non-test code (config.go), so it should be a direct dependency in the main require block. Keeping it under the // indirect section can get reverted by go mod tidy and makes the dependency graph harder to reason about.
|
Lovely. Thank you for the merge. Im not aware of the release schedule of this library. Can I ask for a release? Should I make any changes to the PR for the docs? roadrunner-server/docs#74 |
You're welcome 😃 Is this a feature that you're waiting for? Asking, because I'd prefer to release this in the upcoming (1-2 months) v3.0 (I'm returning to a semver). But if that feature is something that blocking you, I'd be happy to release it with 2025.1.9 Docs PR is good, thank you 👍🏻 |
|
Yes. This feature would help me a lot. I would love to have it as 2025.1.9. <3 |
|
Sure, will be released this week then, thank you for the contribution 🙇🏻 |
Reason for This PR
This PR will fix roadrunner-server/roadrunner#2220. It extends the functionality added in #90.
Description of Changes
This allows us to use glob pattern to specify the path to the proto files. Valid examples are:
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.
PR Checklist
git commit -s).CHANGELOG.md: Add documentation for glob pattern docs#74Summary by CodeRabbit
New Features
{file1,file2}) and recursive directory patterns in configuration.Tests