Fix #5 which prevents tool from running along with other cleanup#10
Open
clach04 wants to merge 5 commits intoJeffCarpenter:masterfrom
Open
Fix #5 which prevents tool from running along with other cleanup#10clach04 wants to merge 5 commits intoJeffCarpenter:masterfrom
clach04 wants to merge 5 commits intoJeffCarpenter:masterfrom
Conversation
Reviewer's GuideThis PR inlines the CLI argument parser into the main entrypoint to preserve existing parameter names, enhances missing-user error handling by printing usage and exiting with an error code, and overhauls the README with clear installation steps, Windows-specific notes, token guidance, and corrected usage examples. Class diagram for updated CLI argument parsing in export_stars.pyclassDiagram
class ArgumentParser {
+add_argument()
+parse_args()
+print_usage()
}
class Args {
+user
+token
}
class main {
+main()
}
ArgumentParser <.. main : uses
Args <.. main : uses
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Author
|
I do have separate branches for some of these if someone wants to cherry-pick. Sharing in one PR for convenience. Thanks for this tool, along with the existing PRs this builds upon 🙏 |
There was a problem hiding this comment.
Hey @clach04 - I've reviewed your changes - here's some feedback:
- The README incorrectly uses "-i requirements.txt" instead of "-r requirements.txt" for installing dependencies and duplicates the install step—please fix and simplify those instructions.
- Consider refactoring the argument parsing back into a separate
parse_args()function for better readability and easier testing rather than inlining it inmain(). - Please add an example in the README showing use of the
--github-tokenflag and/or catchRateLimitExceededExceptionin code to display a more informative error if the API limit is hit.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The README incorrectly uses "-i requirements.txt" instead of "-r requirements.txt" for installing dependencies and duplicates the install step—please fix and simplify those instructions.
- Consider refactoring the argument parsing back into a separate `parse_args()` function for better readability and easier testing rather than inlining it in `main()`.
- Please add an example in the README showing use of the `--github-token` flag and/or catch `RateLimitExceededException` in code to display a more informative error if the API limit is hit.
## Individual Comments
### Comment 1
<location> `readme.md:5` </location>
<code_context>
-Usage: `GH_USER=defunkt python3 export_stars.py > stars.csv`
+Setup:
+
+ python -m pip install -i requirements.txt
+ pip install -i requirements.txt
+
+Usage:
</code_context>
<issue_to_address>
Incorrect pip install flag; should use '-r' instead of '-i' for requirements file.
Replace '-i' with '-r' in both install commands to correctly reference the requirements file.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Author
|
Experiment, @sourcery-ai review as it already found a problem without me asking it to :-) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Apply fixes for parameter name preservation (#5) and missing user error display (#9), streamline argument parsing in main(), and refresh README with installation and usage instructions including Windows and token notes.
Bug Fixes:
--useris missing (spelling mistake in help, repositorys #9)Enhancements:
Documentation: