Skip to content
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

Merged
merged 3 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# mooc.fi CSES exercise task CLI
[![GitHub Actions Workflow Status](https://img.shields.io/github/actions/workflow/status/madeddie/moocfi_cses/test.yml)](https://github.com/madeddie/moocfi_cses/actions)
# 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

Comment on lines +1 to 8

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:

  1. Consistently use new project name for clarity.
  2. 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.

Expand All @@ -14,13 +15,13 @@ This script interacts with the mooc.fi instance of the CSES (https://cses.fi) we
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:
Comment on lines 15 to 27

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.

  1. Confirm the new repository and directory name are correct.
  2. 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

Expand All @@ -34,27 +35,31 @@ This script interacts with the mooc.fi instance of the CSES (https://cses.fi) we
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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CODE REVIEW

Feedback:

  1. Naming Consistency: Ensure the new name tyora.py is updated consistently across the codebase and documentation.
  2. Avoid Informal Tone: The explanation about the origin of the name is informal. Consider a more professional tone for documentation.

Improvements:

  1. Update Example Commands for consistency.
  2. 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".

Expand Down
8 changes: 4 additions & 4 deletions moocfi_cses.py → tyora.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import requests


logger = logging.getLogger(name="moocfi_cses")
logger = logging.getLogger(name="tyora")


@dataclass
Comment on lines 14 to 20

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CODE REVIEW

  1. Meaningful Change Notification: Changing the logger name without context can be confusing. Ensure the reason for the change is documented.

  2. 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.

Expand Down Expand Up @@ -115,7 +115,7 @@ def parse_args(args: Optional[list[str]] = None) -> argparse.Namespace:
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",
Comment on lines 115 to 121

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,
)

Expand Down Expand Up @@ -162,7 +162,7 @@ def create_config() -> dict[str, str]:
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")
Comment on lines 162 to 168

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CODE REVIEW

  1. Logging: Replace print with proper logging for better control and level management.
  2. 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.

Expand Down Expand Up @@ -371,7 +371,7 @@ def main() -> None:
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"
Comment on lines 371 to 377

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CODE REVIEW

  1. Variable Naming: Use more descriptive names for clarity.
  2. 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 = {}

Expand Down