-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Rename project to something more typeable, tyora! #35
Conversation
Warning Rate Limit Exceeded@madeddie has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 51 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe project has transformed through a rebranding initiative, evolving the CLI tool from "mooc.fi CSES exercise task CLI" to "Tyora". This comprehensive alteration encompasses script names, repository links, logger names, and configuration paths to align with the fresh branding. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Feedback from Senior Dev Bot
parser.add_argument( | ||
"--config", | ||
help="Location of config file (default: %(default)s)", | ||
default="~/.config/moocfi_cses/config.json", | ||
default="~/.config/tyora/config.json", | ||
) | ||
parser.add_argument( | ||
"--no-state", |
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.
CODE REVIEW
The code change correctly updates the default config file location. Consider using an all-uppercase variable name for constants to follow PEP 8 guidelines.
DEFAULT_CONFIG_PATH = "~/.config/tyora/config.json"
parser.add_argument(
"--config",
help=f"Location of config file (default: {DEFAULT_CONFIG_PATH})",
default=DEFAULT_CONFIG_PATH,
)
import requests | ||
|
||
|
||
logger = logging.getLogger(name="moocfi_cses") | ||
logger = logging.getLogger(name="tyora") | ||
|
||
|
||
@dataclass |
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.
CODE REVIEW
-
Meaningful Change Notification: Changing the logger name without context can be confusing. Ensure the reason for the change is documented.
-
Consistent Naming: Ensure the new name
tyora
is consistent across the project.
Example update:
import logging
# Updated logger name to `tyora` to better align with the current module
logger = logging.getLogger(name="tyora")
Include comments to explain changes for code clarity.
cookiefile = None | ||
cookies = dict() | ||
if not args.no_state: | ||
state_dir = Path("~/.local/state/moocfi_cses").expanduser() | ||
state_dir = Path("~/.local/state/tyora").expanduser() | ||
if not state_dir.exists(): | ||
state_dir.mkdir(parents=True, exist_ok=True) | ||
cookiefile = state_dir / "cookies.txt" |
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.
CODE REVIEW
- Variable Naming: Use more descriptive names for clarity.
- Path Handling: Consider using
os.path.expanduser
for better readability.
# Improved variable naming and path handling
state_directory = Path(os.path.expanduser("~/.local/state/tyora"))
if not args.no_state:
if not state_directory.exists():
state_directory.mkdir(parents=True, exist_ok=True)
cookiefile = state_directory / "cookies.txt"
cookies = {}
# Tyora: mooc.fi CSES exercise task CLI | ||
[![GitHub Actions Workflow Status](https://img.shields.io/github/actions/workflow/status/madeddie/tyora/test.yml)](https://github.com/madeddie/tyora/actions) | ||
|
||
This script interacts with the mooc.fi instance of the CSES (https://cses.fi) website to perform various actions such as logging in, retrieving exercise lists, and submitting solutions. It provides a convenient way to view and submit tasks. | ||
This script interacts with the mooc.fi instance of the CSES (https://cses.fi) website to perform various actions such as logging in, retrieving exercise lists, and submitting solutions. | ||
It provides a convenient way to view and submit tasks. | ||
|
||
## Features | ||
|
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.
CODE REVIEW
The changes rename the project and update relevant links. The added line break enhances readability.
Suggestions:
- Consistently use new project name for clarity.
- Add a rebranding mention in the project's README.
Revised Example:
-This script interacts with the mooc.fi instance of the CSES (https://cses.fi) website to perform various actions such as logging in, retrieving exercise lists, and submitting solutions.
+`Tyora` interacts with the mooc.fi instance of the CSES (https://cses.fi) website to perform various actions such as logging in, retrieving exercise lists, and submitting solutions.
README.md
Outdated
1. Configure the script by running: | ||
|
||
```bash | ||
python moocfi_cses.py configure | ||
python tyora.py configure | ||
``` | ||
|
||
Follow the prompts to enter your mooc.fi username and password. This information will be stored for future use. | ||
|
||
2. List available exercises: | ||
|
||
```bash | ||
python moocfi_cses.py list | ||
python tyora.py list | ||
``` | ||
|
||
This will retrieve and display a list of exercises available on the CSES platform. | ||
|
||
3. Submit a solution: | ||
|
||
```bash | ||
python moocfi_cses.py submit <exercise_id> <path_to_solution_file> | ||
python tyora.py submit <exercise_id> <path_to_solution_file> | ||
``` | ||
|
||
Replace `<exercise_id>` with the ID of the exercise you want to submit a solution for, and `<path_to_solution_file>` with the path to your solution file. | ||
|
||
## Origin of name | ||
|
||
In Finnish, "työ" means "work", "pyörä" means "wheel". "Työrä" would be "work wheel"? Anyway, `pyora` was already taken, so I went with `tyora`... ;) | ||
|
||
## Contributing | ||
|
||
Contributions are welcome! If you have any suggestions, bug reports, or feature requests, please open an issue or submit a pull request. |
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.
CODE REVIEW
Feedback:
- Naming Consistency: Ensure the new name
tyora.py
is updated consistently across the codebase and documentation. - Avoid Informal Tone: The explanation about the origin of the name is informal. Consider a more professional tone for documentation.
Improvements:
- Update Example Commands for consistency.
- Professional Documentation: Rewrite the origin description formally.
# Origin of name
# The name "tyora" is derived from Finnish words: "työ" meaning "work" and "pyörä" meaning "wheel".
1. Clone the repository to your local machine: | ||
|
||
```bash | ||
git clone https://github.com/madeddie/moocfi_cses.git | ||
git clone https://github.com/madeddie/tyora.git | ||
``` | ||
|
||
2. Navigate to the project directory: | ||
|
||
```bash | ||
cd moocfi_cses | ||
cd tyora | ||
``` | ||
|
||
3. Install the required dependencies: |
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.
CODE REVIEW
The changes are straightforward updates to repository cloning and navigation. However, ensure the repository link and directory name align with project conventions.
- Confirm the new repository and directory name are correct.
- Provide context on why the repository and project name changed.
Example:
- git clone https://github.com/madeddie/moocfi_cses.git
+ git clone https://github.com/madeddie/tyora.git
- cd moocfi_cses
+ cd tyora
def write_config(configfile: str, config: dict[str, str]) -> None: | ||
file_path = Path(configfile).expanduser() | ||
if file_path.exists(): | ||
# TODO: https://github.com/madeddie/moocfi_cses/issues/28 | ||
# TODO: https://github.com/madeddie/tyora/issues/28 | ||
... | ||
file_path.parent.mkdir(parents=True, exist_ok=True) # Ensure directory exists | ||
print("Writing config to file") |
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.
CODE REVIEW
- Logging: Replace
print
with proper logging for better control and level management. - Typing: Use
dict
subscript notation for improved type hints (Python 3.9+).
import logging
from pathlib import Path
logging.basicConfig(level=logging.INFO)
def write_config(configfile: str, config: dict[str, str]) -> None:
file_path = Path(configfile).expanduser()
if file_path.exists():
# TODO: https://github.com/madeddie/tyora/issues/28
...
file_path.parent.mkdir(parents=True, exist_ok=True)
logging.info("Writing config to file")
These changes make the code more robust and up-to-date with best practices.
|
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.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- README.md (3 hunks)
- tyora.py (4 hunks)
Additional comments not posted (10)
README.md (6)
1-2
: LGTM! The project title and GitHub Actions badge link have been updated correctly.
4-5
: LGTM! The project description has been updated correctly.
18-18
: LGTM! The repository clone URL has been updated correctly.
24-24
: LGTM! The directory navigation command has been updated correctly.
38-54
: LGTM! The script usage commands have been updated correctly.
59-61
: LGTM! The new section explaining the origin of the name "Tyora" has been added correctly.tyora.py (4)
17-17
: LGTM! The logger name has been updated correctly.
118-118
: LGTM! The configuration file path has been updated correctly.
165-165
: LGTM! The issue tracker URL has been updated correctly.
374-374
: LGTM! The state directory path has been updated correctly.
|
|
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.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/test_tyora.py (4 hunks)
Additional comments not posted (9)
tests/test_tyora.py (9)
1-1
: Import statement updated correctly.
8-8
: Function call updated correctly.
12-12
: Function call updated correctly.
29-29
: Function call updated correctly.
34-34
: Function call updated correctly.
37-37
: Function call updated correctly.
54-55
: Fixture updated correctly.
62-62
: Function signature updated correctly.
73-73
: Function signature updated correctly.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes