004 GitHub repo selection #6
Conversation
… 004) Add interactive menu [A/S/O/L/Q] when repos.txt is missing or empty: - [A] Analyze ALL accessible repositories - [S] Specify repository names manually (owner/repo format) - [O] Analyze organization repositories - [L] Select from list by number (e.g., 1,3,5 or 1-3) - [Q] Quit/Skip GitHub analysis Implementation includes: - GitHubClient.list_user_repos() with pagination support - GitHubClient.list_org_repos() with pagination support - validate_repo_format() and validate_org_name() helpers - format_repo_list() for numbered display - load_github_repos_from_file() for repos.txt parsing - Rate limit handling with wait time display - EOF/KeyboardInterrupt graceful handling - Non-interactive mode support (--quiet flag) Tests: 727 passed (24 new GitHub selection tests)
The select_github_repos() function was implemented but not integrated into main(). This caused the interactive menu to never appear when repos.txt was missing/empty. Changes: - Replace load_repositories() with select_github_repos() in main() - Convert returned repo names (strings) to Repository objects - Update unit tests to mock select_github_repos instead of load_repositories - Add required github_token and repos_file attributes to mock configs
GitHub allows repository names that start with a period (e.g., .github, .dotfiles). Updated REPO_COMPONENT_PATTERN to accept names starting with alphanumeric OR period. This fixes the "Invalid repository name format" error when selecting repositories from the GitHub API.
Summary of ChangesHello @amargiovanni, 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 delivers a significant enhancement to the GitHub analysis tool by introducing an interactive repository selection mechanism. This feature improves user experience by providing flexible options for choosing repositories directly from the command line, aligning with the UX pattern of existing Jira project selection. The changes are backed by a comprehensive design and testing process, ensuring a robust and user-friendly addition. 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 introduces a significant and well-designed feature for interactive GitHub repository selection. The changes are thoroughly documented with specifications, contracts, and checklists, which is excellent practice. The implementation is mostly solid, with comprehensive test coverage for the new functionality. I've identified a high-severity bug where user configuration is ignored during API calls, a medium-severity issue with brittle error handling, and a style violation. Once these are addressed, this will be a great addition to the tool.
| def select_github_repos( | ||
| repos_file: str, | ||
| github_token: str, | ||
| interactive: bool = True, | ||
| output: TerminalOutput | None = None, | ||
| ) -> list[str]: | ||
| """Select GitHub repositories from file or interactively (Feature 004). | ||
| Per spec FR-001 to FR-014: | ||
| - Display interactive menu when repos.txt is missing or empty | ||
| - Options: [A] All personal, [S] Specify manually, [O] Organization, | ||
| [L] Select from list, [Q] Quit/Skip | ||
| - Follow select_jira_projects pattern for UX consistency (FR-003) | ||
| Args: | ||
| repos_file: Path to repos.txt file. | ||
| github_token: GitHub API token for API calls. | ||
| interactive: If True, prompt user when file is missing/empty. | ||
| If False (--quiet mode), skip prompts per FR-013. | ||
| output: Optional TerminalOutput for consistent logging. | ||
| Returns: | ||
| List of repository names (owner/repo format) to analyze. | ||
| """ | ||
| # Helper for consistent output | ||
| def log(msg: str, level: str = "info") -> None: | ||
| if output: | ||
| output.log(msg, level) | ||
| else: | ||
| print(msg) | ||
|
|
||
| # Try loading from file first (FR-001) | ||
| file_repos = load_github_repos_from_file(repos_file) | ||
| if file_repos: | ||
| return file_repos | ||
|
|
||
| # No file or empty - need to prompt or skip | ||
| if not interactive: | ||
| # FR-013, FR-014: Non-interactive mode skips prompts | ||
| log("No repos.txt found. Skipping GitHub analysis in non-interactive mode.", "info") | ||
| return [] | ||
|
|
||
| # Display menu per FR-002 | ||
| print("\nOptions:") | ||
| print(" [A] Analyze ALL accessible repositories") | ||
| print(" [S] Specify repository names manually (owner/repo format)") | ||
| print(" [O] Analyze organization repositories") | ||
| print(" [L] Select from list by number (e.g., 1,3,5 or 1-3)") | ||
| print(" [Q] Quit/Skip GitHub analysis") | ||
|
|
||
| # Create client for API calls - use provided token | ||
| config = AnalyzerConfig(github_token=github_token) | ||
| client = GitHubClient(config) |
There was a problem hiding this comment.
The select_github_repos function is re-creating its own AnalyzerConfig object using only the github_token. This causes any other user-defined settings from the environment or config (like timeout, per_page, and max_pages) to be ignored and fall back to defaults. This can lead to unexpected behavior, such as premature timeouts or incomplete data from pagination.
The function should accept the fully-populated config object from main() and use that to initialize the GitHubClient.
After applying this suggestion, you'll also need to update the call to this function in main() at line 1004 to pass the config object instead of repos_file and github_token separately:
repo_names = select_github_repos(
config=config,
interactive=interactive,
output=output,
)def select_github_repos(
config: AnalyzerConfig,
interactive: bool = True,
output: TerminalOutput | None = None,
) -> list[str]:
"""Select GitHub repositories from file or interactively (Feature 004).
Per spec FR-001 to FR-014:
- Display interactive menu when repos.txt is missing or empty
- Options: [A] All personal, [S] Specify manually, [O] Organization,
[L] Select from list, [Q] Quit/Skip
- Follow select_jira_projects pattern for UX consistency (FR-003)
Args:
config: The main analyzer configuration object.
interactive: If True, prompt user when file is missing/empty.
If False (--quiet mode), skip prompts per FR-013.
output: Optional TerminalOutput for consistent logging.
Returns:
List of repository names (owner/repo format) to analyze.
"""
# Helper for consistent output
def log(msg: str, level: str = "info") -> None:
if output:
output.log(msg, level)
else:
print(msg)
# Try loading from file first (FR-001)
file_repos = load_github_repos_from_file(config.repos_file)
if file_repos:
return file_repos
# No file or empty - need to prompt or skip
if not interactive:
# FR-013, FR-014: Non-interactive mode skips prompts
log("No repos.txt found. Skipping GitHub analysis in non-interactive mode.", "info")
return []
# Display menu per FR-002
print("\nOptions:")
print(" [A] Analyze ALL accessible repositories")
print(" [S] Specify repository names manually (owner/repo format)")
print(" [O] Analyze organization repositories")
print(" [L] Select from list by number (e.g., 1,3,5 or 1-3)")
print(" [Q] Quit/Skip GitHub analysis")
# Create client for API calls
client = GitHubClient(config)| e: RateLimitError with reset_time. | ||
| log: Logging function. | ||
| """ | ||
| import time as time_module |
There was a problem hiding this comment.
According to the PEP 8 style guide, imports should be at the top of the file, not inside a function. This local import of time makes the code harder to read and maintain.
Please move this import to the top of the file as import time, and then change time_module.time() to time.time() in this function.
| except GitHubAnalyzerError as e: | ||
| if "404" in str(e): | ||
| log(f"Organization '{org_name}' not found or not accessible.", "warning") | ||
| else: | ||
| log(f"Error fetching organization repos: {e.message}", "error") | ||
| continue |
There was a problem hiding this comment.
Checking for a 404 error by converting the exception to a string and searching for "404" is brittle. The error message could change, or another part of the exception's string representation might contain "404" for a different reason.
A more robust approach is to check if the exception is an APIError and then inspect its status_code attribute directly.
| except GitHubAnalyzerError as e: | |
| if "404" in str(e): | |
| log(f"Organization '{org_name}' not found or not accessible.", "warning") | |
| else: | |
| log(f"Error fetching organization repos: {e.message}", "error") | |
| continue | |
| except GitHubAnalyzerError as e: | |
| # APIError is a subclass of GitHubAnalyzerError and has a status_code | |
| if hasattr(e, "status_code") and e.status_code == 404: | |
| log(f"Organization '{org_name}' not found or not accessible.", "warning") | |
| else: | |
| log(f"Error fetching organization repos: {e.message}", "error") | |
| continue |
No description provided.