Skip to content

chore: config feedback #610

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

Merged
merged 12 commits into from
Feb 22, 2025
Merged

chore: config feedback #610

merged 12 commits into from
Feb 22, 2025

Conversation

caroljung-cg
Copy link
Contributor

@caroljung-cg caroljung-cg commented Feb 21, 2025

No description provided.

@caroljung-cg caroljung-cg requested review from codegen-team and a team as code owners February 21, 2025 22:45
@codegen-sh codegen-sh deleted a comment from CLAassistant Feb 21, 2025
@codegen-sh codegen-sh deleted a comment from codecov bot Feb 21, 2025
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


codegen-bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codegen-team
Copy link
Contributor

Consider adding docstring updates to clarify the new configuration initialization patterns, especially in key classes like CodebaseContext and Codebase. This would help developers understand when and how to provide custom configurations vs letting them default.

@codegen-team
Copy link
Contributor

The new environment variable override behavior in GlobalEnv._load_dotenv() could benefit from a comment explaining the precedence rules, especially since override=True is now being used. This helps maintain clarity about which values take precedence when multiple sources define the same variable.

@codegen-team
Copy link
Contributor

The moved get_git_root_path function in shared/path.py could be enhanced with error handling for common edge cases:

  1. When git is not installed
  2. When the path is in a git worktree
  3. When the path contains symlinks

This would make the utility more robust for various development environments.

@codegen-team
Copy link
Contributor

While the changes look solid, consider adding specific test cases for:

  1. Configuration initialization with and without explicit parameters
  2. Environment variable precedence with the new override behavior
  3. Edge cases in git root path detection

This would help ensure the refactoring remains stable across future changes.

Copy link

codecov bot commented Feb 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files

@caroljung-cg caroljung-cg merged commit b5ef76e into develop Feb 22, 2025
24 of 26 checks passed
@caroljung-cg caroljung-cg deleted the load-dotenv branch February 22, 2025 00:03
Copy link
Contributor

🎉 This PR is included in version 0.32.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants