-
Notifications
You must be signed in to change notification settings - Fork 0
Enhance current commit resolution logic in GitUtils with fallback #77
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
Conversation
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.
Pull Request Overview
Enhances the current commit resolution logic in GitUtils by adding a fallback mechanism to improve reliability when resolving Git references. The change adds error handling and alternative resolution strategies when the primary reference resolution fails.
- Implements nested try-catch blocks for robust current commit resolution
- Adds fallback logic to try remote branch format when local branch resolution fails
- Includes debug and warning logging for better troubleshooting
self.logger.debug( | ||
f"Successfully resolved current ref: {git_info.current_ref}" | ||
) | ||
except Exception: |
Copilot
AI
Jul 23, 2025
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.
Catching generic Exception is too broad. Consider catching specific Git-related exceptions like GitCommandError or BadName to avoid masking unexpected errors.
except Exception: | |
except BadName: |
Copilot uses AI. Check for mistakes.
) | ||
self.logger.debug(f"Trying remote ref: {remote_ref}") | ||
current_commit = self.repo.commit(remote_ref) | ||
except Exception as e: |
Copilot
AI
Jul 23, 2025
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.
Catching generic Exception is too broad. Consider catching specific Git-related exceptions like GitCommandError or BadName to avoid masking unexpected errors.
except Exception as e: | |
except (GitCommandError, BadName) as e: |
Copilot uses AI. Check for mistakes.
"refs/heads/", "origin/" | ||
) | ||
self.logger.debug(f"Trying remote ref: {remote_ref}") | ||
current_commit = self.repo.commit(remote_ref) |
Copilot
AI
Jul 23, 2025
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.
This line can raise an exception that won't be caught by the inner try-catch block at line 147, potentially causing the outer catch block to trigger unexpectedly. The remote_ref resolution should be wrapped in its own try-catch block.
current_commit = self.repo.commit(remote_ref) | |
try: | |
current_commit = self.repo.commit(remote_ref) | |
except Exception as e: | |
self.logger.debug( | |
f"Failed to resolve remote ref '{remote_ref}': {e}" | |
) | |
raise |
Copilot uses AI. Check for mistakes.
"refs/pull" | ||
): | ||
# Resolve current commit with fallback logic | ||
try: |
Copilot
AI
Jul 23, 2025
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.
[nitpick] The nested try-catch structure creates complex control flow. Consider extracting the commit resolution logic into a separate method to improve readability and maintainability.
Copilot uses AI. Check for mistakes.
…dling