chore: Rename Make* constructors to New* #149
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR renames exported constructor/factory functions from the MakeX naming pattern to NewX across the codebase, updating all call sites, tests, and documentation to use the New* identifiers. No behavioral, parameter, or return-type changes are introduced. Changes
Sequence Diagram(s)(omitted — changes are identifier renames across existing control flows; control flow itself is unchanged) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Reasoning: Widespread but homogeneous renames across many packages and tests; review requires ensuring no symbol was missed and that public API consumers are aligned, but logic density per file is low. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Summary of ChangesHello @gocanto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a significant refactoring effort to standardize constructor naming conventions throughout the Go codebase. By consistently using the 'New' prefix for constructor functions, the change aims to improve code readability, maintainability, and adherence to Go best practices. This is a purely cosmetic and organizational change, with no alteration to the underlying logic or functionality of the application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great initiative to align the codebase with idiomatic Go conventions by renaming Make* constructors to New*. The changes are extensive and have been applied consistently across the codebase, including source files, tests, and documentation. I've only found a few minor inconsistencies in test error messages that could be updated for full consistency. Overall, this is a solid and well-executed refactoring.
gocanto
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
database/seeder/importer/runner_test.go (1)
714-716: LGTM!The constructor call and error message have been correctly updated to reflect the new naming convention.
metal/cli/seo/testhelpers_test.go (1)
105-107: LGTM!The constructor call and error message have been correctly updated to reflect the new naming convention.
database/seeder/seeds/seeder_test.go (1)
64-67: LGTM!The constructor call and error message have been correctly updated to reflect the new naming convention.
database/connection_test.go (1)
72-74: LGTM!The constructor call and error message have been correctly updated to reflect the new naming convention.
database/repository/repository_test.go (1)
68-71: LGTM!The constructor call and error message have been correctly updated to reflect the new naming convention.
metal/cli/clitest/helpers.go (1)
63-65: LGTM! Call site and error message correctly updated.Both the function call and error message have been updated to reflect the new
NewConnectionnaming.metal/cli/accounts/handler_test.go (1)
11-15: LGTM! Test setup correctly updated.All constructor calls and the error message have been updated to use the new
New*naming convention.metal/cli/accounts/factory_test.go (1)
10-15: LGTM! Test function and setup correctly updated.The test name, constructor calls, and error message have all been updated to reflect the new
New*naming convention.
🧹 Nitpick comments (1)
pkg/media/media_test.go (1)
28-28: Optional: Consider more descriptive error messages.While "new: %v" is consistent with the function rename, slightly more descriptive messages could improve test failure diagnostics.
Example alternatives:
- t.Fatalf("new: %v", err) + t.Fatalf("NewMedia failed: %v", err)Or:
- t.Fatalf("new: %v", err) + t.Fatalf("failed to create media: %v", err)Note: The current error messages have already been updated per previous review suggestions, so this is purely a nice-to-have refinement.
Also applies to: 79-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
database/connection_test.go(1 hunks)database/repository/repository_test.go(1 hunks)database/repository/testhelpers_test.go(1 hunks)database/seeder/importer/runner_test.go(1 hunks)database/seeder/seeds/seeder_test.go(2 hunks)handler/tests/db.go(2 hunks)metal/cli/accounts/factory_test.go(2 hunks)metal/cli/accounts/handler_test.go(1 hunks)metal/cli/clitest/helpers.go(3 hunks)metal/cli/seo/testhelpers_test.go(1 hunks)pkg/auth/handler_test.go(4 hunks)pkg/llogs/files_logs_test.go(1 hunks)pkg/media/media_test.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- database/repository/testhelpers_test.go
- handler/tests/db.go
🧰 Additional context used
🧬 Code graph analysis (11)
pkg/llogs/files_logs_test.go (1)
pkg/llogs/files_logs.go (1)
NewFilesLogs(19-44)
database/seeder/importer/runner_test.go (1)
database/connection.go (1)
NewConnection(19-32)
metal/cli/accounts/factory_test.go (3)
metal/cli/clitest/helpers.go (2)
NewTestConnection(16-76)NewTestEnv(78-80)database/model.go (1)
APIKey(27-39)metal/cli/accounts/factory.go (1)
NewHandler(19-34)
metal/cli/seo/testhelpers_test.go (1)
database/connection.go (1)
NewConnection(19-32)
pkg/media/media_test.go (1)
pkg/media/media.go (1)
NewMedia(30-58)
database/repository/repository_test.go (1)
database/connection.go (1)
NewConnection(19-32)
pkg/auth/handler_test.go (1)
pkg/auth/handler.go (1)
NewTokensHandler(16-26)
database/connection_test.go (1)
database/connection.go (1)
NewConnection(19-32)
metal/cli/clitest/helpers.go (2)
database/connection.go (2)
Connection(12-17)NewConnection(19-32)metal/env/env.go (1)
Environment(9-17)
metal/cli/accounts/handler_test.go (2)
metal/cli/clitest/helpers.go (2)
NewTestConnection(16-76)NewTestEnv(78-80)metal/cli/accounts/factory.go (1)
NewHandler(19-34)
database/seeder/seeds/seeder_test.go (2)
database/connection.go (1)
NewConnection(19-32)database/seeder/seeds/factory.go (1)
NewSeeder(17-22)
🔇 Additional comments (8)
pkg/llogs/files_logs_test.go (2)
19-19: LGTM! Constructor renaming is correct.The function call has been properly updated from
MakeFilesLogstoNewFilesLogs, consistent with the PR's renaming objectives. The function signature matches the implementation shown in the relevant code snippets.
22-22: LGTM! Error message updated to match new function name.The error message has been appropriately updated from "make logs" to "new logs", addressing the previous review feedback and maintaining consistency with the renamed constructor.
pkg/auth/handler_test.go (1)
12-12: LGTM! Rename refactoring completed consistently.All constructor calls and error messages have been correctly updated from
Make*toNew*throughout the test file. The test function nameTestNewTokensHandlerError(line 35) also reflects the new naming convention. The changes align with the PR objective and the past review comments have been addressed.Also applies to: 15-15, 35-36, 50-50, 53-53, 74-74, 77-77
database/seeder/seeds/seeder_test.go (1)
95-95: LGTM!The seeder factory call has been correctly updated from
MakeSeedertoNewSeeder, consistent with the codebase-wide renaming pattern.metal/cli/clitest/helpers.go (2)
16-16: LGTM! Function rename follows Go conventions.The rename from
MadeTestConnectiontoNewTestConnectionaligns with idiomatic Go constructor naming patterns.
78-80: LGTM! Function rename follows Go conventions.The rename from
MadeTestEnvtoNewTestEnvaligns with idiomatic Go constructor naming patterns.metal/cli/accounts/factory_test.go (1)
33-40: LGTM! Test function correctly updated.The test name and all constructor calls have been updated to use the new
New*naming convention.pkg/media/media_test.go (1)
21-87: LGTM! Rename is complete and consistent.All references to
MakeMediahave been correctly updated toNewMediathroughout the test file, including:
- Test function names (
TestNewMediaAndUpload,TestNewMediaErrors)- All function calls with correct signature matching
- Error messages updated to reflect the new constructor name
The changes follow Go naming conventions and previous review feedback has been implemented.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f0a27ee468833381cd365441b9cada
Summary by CodeRabbit