feat(model): enable key features by default#245
Conversation
Enable data masking, encryption, CCUsage, AICodeOtel, and code tracking by default so new users get the full feature set out of the box. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello, 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 streamlines the initial setup experience by activating several key features by default within the Highlights
Changelog
Activity
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
|
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request enables several key features by default for new installations by updating DefaultConfig. The changes align with the goal of providing a better out-of-the-box experience.
My review includes two main points:
- Inconsistent default handling: There's an inconsistency in how default settings are applied. While
DataMaskinghas a fallback for existing users, the other newly-enabled features do not. I've recommended aligning the approach for all features to ensure consistency and to provide these new defaults to all users who haven't explicitly configured them. - Magic number: A hardcoded port number should be replaced with a constant to improve maintainability.
Overall, the changes are positive, and addressing these points would further improve the codebase's robustness and consistency.
| DataMasking: new(true), | ||
| Endpoints: nil, | ||
| EnableMetrics: nil, | ||
| Encrypted: nil, | ||
| Encrypted: new(true), | ||
| AI: DefaultAIConfig, | ||
| Exclude: []string{}, | ||
| CCUsage: nil, | ||
| CCOtel: nil, // deprecated | ||
| AICodeOtel: nil, | ||
| CodeTracking: nil, | ||
| LogCleanup: nil, | ||
| CCUsage: new(CCUsage{ | ||
| Enabled: new(true), | ||
| }), | ||
| CCOtel: nil, | ||
| AICodeOtel: new(AICodeOtel{ | ||
| Enabled: new(true), | ||
| GRPCPort: 54027, | ||
| Debug: new(false), | ||
| }), | ||
| CodeTracking: new(CodeTracking{ | ||
| Enabled: new(true), | ||
| }), |
There was a problem hiding this comment.
These changes enable several features by default in DefaultConfig, which is great for new installations. However, the way defaults are applied seems inconsistent.
In model/config.go, DataMasking is already defaulted to true if it's nil when reading a config. This ensures the feature is on for existing users who haven't configured it.
The other features enabled in this PR (Encrypted, CCUsage, AICodeOtel, CodeTracking) don't have this fallback logic in ReadConfigFile. This means they will be enabled for new users, but will remain disabled (nil) for existing users who don't have these settings in their config files.
For consistency and to provide the benefits of these features (especially security-related ones like Encrypted) to all users, consider adding similar fallback logic for these new default-enabled features in model/config.go's ReadConfigFile function, similar to how DataMasking is handled.
| CCOtel: nil, | ||
| AICodeOtel: new(AICodeOtel{ | ||
| Enabled: new(true), | ||
| GRPCPort: 54027, |
There was a problem hiding this comment.
The gRPC port 54027 is hardcoded here and also in model/config.go:275. To avoid magic numbers and ensure consistency, it's better to define this as a constant, for example const DefaultAICodeOtelGRPCPort = 54027 at the package level, and use it in both places.
| GRPCPort: 54027, | |
| GRPCPort: DefaultAICodeOtelGRPCPort, |
Summary
Test plan
config.local.tomloverrides are not affectedgo test ./model/...to ensure no regressions🤖 Generated with Claude Code