Conversation
Codecov Report❌ Patch coverage is |
![codegen-sh[bot]](https://avatars.githubusercontent.com/in/321279?s=60&v=4){kind=small}
|
|
||
| # Base directories | ||
| CONFIG_DIR = Path("~/.config/codegen-sh").expanduser() | ||
| CONFIG_DIR = Path("~/.codegen").expanduser() |
There was a problem hiding this comment.
[Logic bug]: CONFIG_DIR path change breaks consistency
Other modules (e.g. src/codegen/configs/constants.py) still reference the old ~/.config/codegen-sh directory, so credentials/config written here will no longer be found elsewhere.
| CONFIG_DIR = Path("~/.codegen").expanduser() | |
| # Re-align with the existing global constants – keep the old location or update all usages consistently | |
| CONFIG_DIR = Path("~/.config/codegen-sh").expanduser() |
| rich.print(f"[green]✓ Stored token to:[/green] {token_manager.token_file}") | ||
| rich.print("[cyan]📊 Hey![/cyan] We collect anonymous usage data to improve your experience 🔒") | ||
| rich.print("To opt out, set [green]telemetry_enabled = false[/green] in [cyan]~/.config/codegen-sh/analytics.json[/cyan] ✨") | ||
| return token |
There was a problem hiding this comment.
[Logic bug]: Removed consent message may hide telemetry notice
Deleting the opt-out instructions lines means users are no longer told how to disable telemetry.
| return token | |
| rich.print("[cyan]📊 Hey![/cyan] We collect anonymous usage data to improve your experience 🔒") | |
| rich.print("To opt out, set [green]telemetry_enabled = false[/green] in [cyan]~/.config/codegen-sh/analytics.json[/cyan] ✨") |
| # Import config command (still a Typer app) | ||
| from codegen.cli.commands.config.main import config_command | ||
| from codegen.cli.commands.init.main import init | ||
| from codegen.cli.commands.integrations.main import integrations_app |
There was a problem hiding this comment.
[Syntax error]: extraneous import removed in diff but still referenced
codegen/cli/cli.py still imports codegen.cli.commands.mcp.main which has been deleted, causing ImportError at runtime.
| from codegen.cli.commands.integrations.main import integrations_app | |
| # from codegen.cli.commands.mcp.main import mcp # removed command |
| main = typer.Typer(name="codegen", help="Codegen - the Operating System for Code Agents.", rich_markup_mode="rich") | ||
|
|
||
| # Add individual commands to the main app | ||
| main.command("claude", help="Run Claude Code with OpenTelemetry monitoring and logging.")(claude) |
There was a problem hiding this comment.
[Logic error]: Command registration for removed mcp still present
Since mcp function is no longer imported, these lines will raise NameError when CLI module is imported.
| main.command("claude", help="Run Claude Code with OpenTelemetry monitoring and logging.")(claude) | |
| # main.command("mcp", help="Start the Codegen MCP server.")(mcp) # command removed |
|
|
||
| # Should contain basic help text | ||
| assert "Codegen CLI - Transform your code with AI" in result.stdout | ||
| assert "Commands" in result.stdout |
There was a problem hiding this comment.
[Test failure]: Unit tests still assert old help string
Help message string changed; update tests or revert help text to keep tests passing.
| assert "Commands" in result.stdout | |
| assert "Codegen - the Operating System for Code Agents" in result.stdout |
| rich.print("[cyan]📊 Hey![/cyan] We collect anonymous usage data to improve your experience 🔒") | ||
| rich.print("To opt out, set [green]telemetry_enabled = false[/green] in [cyan]~/.config/codegen-sh/analytics.json[/cyan] ✨") | ||
| return token | ||
| except AuthError as e: |
There was a problem hiding this comment.
[Logic error]: Telemetry opt-out path hard-coded to old config location
If CONFIG_DIR path is changed, this message must also reflect new location.
| except AuthError as e: | |
| rich.print("To opt out, set [green]telemetry_enabled = false[/green] in [cyan]~/.codegen/analytics.json[/cyan] ✨") |
|
Found 7 issues. Please review my inline comments above. |
|
🎉 This PR is included in version 0.56.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
2 participants
Motivation
Content
Testing
Please check the following before marking your PR as ready for review