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

feat: add summary for easier usage #426

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Conversation

peter-jerry-ye
Copy link
Collaborator

Add summary for each section. User may choose to download them for prompting LLMs.

Copy link

peter-jerry-ye-code-review bot commented Jan 16, 2025

‼️ This code review is generated by a bot. Please verify the content before trusting it.

Here are three potential issues or observations from the provided git diff output:

  1. Python Version in .readthedocs.yaml:

    • The Python version specified in .readthedocs.yaml is "3.13". As of now, Python 3.13 is not a released version. This could cause issues during the build process if the specified version is not available. Consider using a stable and released version of Python, such as "3.11" or "3.12".
  2. No Newline at End of File:

    • Several files, such as next/llm.py, next/.gitignore, and next/tutorial/index.md, are missing a newline at the end of the file. While this is not a critical issue, it is a good practice to ensure that all files end with a newline character to avoid potential issues with certain tools and text editors.
  3. Hardcoded Paths in llm.py:

    • The llm.py script contains hardcoded paths like BUILD_DIR="_build/markdown" and output_file = f"download/{directory}/summary.md". This could lead to issues if the script is run in a different environment or if the directory structure changes. Consider making these paths configurable or relative to the script's location to improve flexibility and maintainability.

These are the most notable observations from the diff. Addressing these issues could improve the robustness and maintainability of the codebase.

@peter-jerry-ye peter-jerry-ye force-pushed the zihang/llm-summary branch 3 times, most recently from 96cbd67 to 1f3b7b4 Compare January 16, 2025 07:42
@peter-jerry-ye peter-jerry-ye merged commit cb21840 into main Jan 16, 2025
4 checks passed
@peter-jerry-ye peter-jerry-ye deleted the zihang/llm-summary branch January 16, 2025 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant