-
Notifications
You must be signed in to change notification settings - Fork 137
Support batch mode for better performance on large codebases #244
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces a new “Batch mode” for the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as mockgen Main
participant PT as prepareTargets
participant GT as generateTarget
participant G as generator
U->>M: Run mockgen with YAML config
M->>PT: Process YAML to extract batch targets
PT-->>M: Return list of genTarget objects
loop For each target
M->>GT: Call generateTarget(target)
GT->>G: Invoke Generate(target) / GenerateMockInterface(target)
G-->>GT: Return generated mock
end
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
README.md (1)
79-80
: Minor grammatical improvement
Add a comma for clarity.- To use batch mode you need to prepare a YAML file. + To use batch mode, you need to prepare a YAML file.🧰 Tools
🪛 LanguageTool
[uncategorized] ~80-~80: Possible missing comma found.
Context: ...neration process shorter. To use batch mode you need to prepare a YAML file. The fi...(AI_HYDRA_LEO_MISSING_COMMA)
mockgen/package_mode.go (2)
43-46
: Improve field naming
The fieldsname
andifaces
inparseTarget
work, but considerpkgName
or another more explicit identifier for clarity in large codebases.
95-168
: Alias detection breadth
The iteration over package files to track type alias usage covers potential edge cases. If performance becomes a concern for extremely large codebases, exploring concurrency or caching might help.mockgen/mockgen.go (4)
82-83
: Batch mode flag usage.Providing the
-batch
flag simplifies specifying multiple targets at once. Consider logging a warning if other flags (like-source
) are set simultaneously, since the batch mode supersedes them.
116-129
: Possibility to further splitprepareTargets
.While this function clearly enumerates different modes (modelGob, source, batch, default package), it’s somewhat lengthy. Consider extracting each piece (e.g., reading from gob, reading from source, etc.) into smaller private helpers to simplify maintenance.
245-245
: NewgenerateTarget
function.Extracting this target-based generation logic is helpful for maintainability and reusability. Consider parallelizing multiple calls for performance gains if large numbers of targets are encountered.
612-612
: Generate directive support.Conditionally writing a
//go:generate
directive is a nice additive feature. Be mindful that the command might include sensitive environment variables or tokens if used in a CI environment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md
(1 hunks)go.mod
(1 hunks)mockgen/mockgen.go
(16 hunks)mockgen/mockgen_test.go
(1 hunks)mockgen/package_mode.go
(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
mockgen/package_mode.go (1)
mockgen/model/model.go (1)
Package
(30-35)
mockgen/mockgen.go (1)
mockgen/model/model.go (2)
Package
(30-35)Interface
(58-62)
🪛 LanguageTool
README.md
[uncategorized] ~80-~80: Possible missing comma found.
Context: ...neration process shorter. To use batch mode you need to prepare a YAML file. The fi...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (22)
go.mod (1)
9-9
:✅ Verification successful
Verify direct dependency usage
Now thatgopkg.in/yaml.v3
is a direct dependency, ensure that any consumers will not be affected by version mismatches or security advisories.
🏁 Script executed:
#!/bin/bash # Check security vulnerability info for gopkg.in/yaml.v3 gh api graphql -f query=' { securityVulnerabilities(ecosystem: GO, first:10, package:"gopkg.in/yaml.v3") { nodes { advisory { summary severity } vulnerableVersionRange } } }'Length of output: 414
Dependency usage verified
- The
go.mod
file now listsgopkg.in/yaml.v3 v3.0.1
as a direct dependency.- The GitHub GraphQL API output confirms a known high-severity vulnerability ("gopkg.in/yaml.v3 Denial of Service") affects versions lower than
3.0.0-20220521103104-8f96da9f5d5e
.- Since version v3.0.1 is installed, it is beyond the vulnerable range, ensuring that consumers should not be impacted by this advisory.
mockgen/mockgen_test.go (1)
248-248
: Validate new boolean parameter usage
You've added a boolean argument (false
) toGenerateMockInterface
. Verify that the test suite includes a scenario fortrue
as well, to confirm correctness and maintain coverage for both code paths.Do you need any assistance ensuring this new parameter is thoroughly tested across all relevant scenarios?
README.md (1)
74-99
: Great documentation for Batch mode
The new section lays out the YAML configuration usage clearly, which should help users adopt batch processing more easily.🧰 Tools
🪛 LanguageTool
[uncategorized] ~80-~80: Possible missing comma found.
Context: ...neration process shorter. To use batch mode you need to prepare a YAML file. The fi...(AI_HYDRA_LEO_MISSING_COMMA)
mockgen/package_mode.go (3)
35-41
: Refactored single-package logic
SwitchingparsePackage
to delegate toparsePackages
nicely consolidates single- and multi-package parsing logic, reducing duplication.
48-84
: parsePackages method observation
The approach of collecting all package names first, loading them, and then extracting interfaces per package is well-structured. Error handling also appears robust.
170-192
: Comprehensive error aggregation
Employingerrors.Join
is an excellent way to surface multiple package load issues simultaneously, aiding quicker debugging.🧰 Tools
🪛 golangci-lint (1.64.8)
177-177: SA1019: packages.LoadSyntax is deprecated: LoadSyntax exists for historical compatibility and should not be used. Please directly specify the needed fields using the Need values.
(staticcheck)
mockgen/mockgen.go (16)
42-42
: Consider pinning the YAML dependency in go.mod.Introducing
"gopkg.in/yaml.v3"
here is important for batch processing. Ensure the dependency is explicitly pinned in yourgo.mod
to prevent unexpected breakage if the YAML package updates in the future.
57-63
: Well-defined default values.The constants for default behavior (
defaultWriteCmdComment
, etc.) enhance clarity and reduce duplication across flag definitions. This is a good approach for maintainability.
71-78
: Flags consistency.Using the newly introduced defaults in the flag definitions is consistent and ensures a single source of truth for default values. No issues noted.
99-104
: Modular entry point.Delegating target setup to
prepareTargets()
is a clean separation of concerns. This improves readability in the main entry point.
131-146
: Source & modelGob modes.Both branches properly handle file reading & package parsing. The error handling via
log.Fatalf
is acceptable for a CLI. Great job ensuring a singlegenTarget
is returned for these modes.
147-196
: Batch-mode logic.Parsing YAML into a batch configuration and mapping each target is well-organized. The usage of
strings.Cut
(line 159) requires Go 1.18 or higher. Confirm your Go version constraints or gracefully handle older versions if needed.Would you like me to check the codebase or documentation for the minimum Go version using a script?
225-233
: Boolean override strategy.Using
overrideBool
to layer defaults, global, and package-level settings is straightforward and avoids confusion about priority. This is a solid design.
235-243
: String override strategy.Similar to
overrideBool
,overrideString
is equally robust in layering configuration. No issues noted.
275-275
: Handling build constraints.Passing
buildConstraint
to the new generator is direct and keeps the constraint logic centralized. This is a neat extension of the generator struct.
296-296
: Centralized generation call.Switching to
g.Generate(target, ...)
rather than multiple parameters simplifies the generator’s signature and ensures future expansions stay consistent.
301-301
: Conditional directory creation.Creating the destination directory only if a non-empty destination is specified is an elegant approach. This avoids polluting the filesystem when writing to stdout.
603-603
: Dot import usage.Including dot-imports ensures consistent referencing of those packages within generated mocks. This preserves the original package semantics and avoids potential naming collisions.
613-613
: Typed parameter injection.Passing
target.typed
toGenerateMockInterface
smoothly integrates typed mocking logic. Good separation of concerns with thetyped
field ingenTarget
.
696-718
: Typed mock methods.The code conditionally invokes typed mocking methods (
GenerateMockRecorderMethod
,GenerateMockReturnCallMethod
) whentyped
is enabled. This design scales well for future advanced mocking features.
827-829
: Branched call type.Distinguishing between typed and standard
*gomock.Call
ensures backward compatibility while offering improved type safety for advanced usage. Clean approach.
855-857
: Typed call chaining.Generating a specialized
*%s%sCall%s
struct that chains typed calls is a robust extension of the standardgomock.Call
. This is an intuitive interface for type-safe mocks.
See #245 for the motivation behind this PR.
I split this PR into a series of self-contained commits, so that it's easier to review the changes commit-by-commit.
Summary by CodeRabbit
Documentation
New Features