Skip to content

Conversation

@NeoZhangJianyu
Copy link
Contributor

@NeoZhangJianyu NeoZhangJianyu commented Nov 5, 2025

User description

Type of Change

bug fix

Description

In examples/3.x_api/README.md, the link need to be added README.md for html.
This script fix this issue in history release.

Expected Behavior & Potential Risk

How has this PR been tested?

verify locally.

Dependency Change?

NA


PR Type

Enhancement, Bug fix


Description

  • Add script to fix missing README links

  • Update build script to include examples directory


Diagram Walkthrough

flowchart LR
  A["Add add_readme.py script"] -- "Fixes missing README links" --> B["Update build.sh"]
  B -- "Includes examples directory" --> C["Build process updated"]
Loading

File Walkthrough

Relevant files
Enhancement
add_readme.py
Add script to fix README links                                                     

docs/build_docs/add_readme.py

  • Added script to fix missing README links in markdown files
  • Reads and modifies files in place to add '/README.md' to href links
+32/-0   
build.sh
Update build script to include examples                                   

docs/build_docs/build.sh

  • Updated to include examples directory in build process
  • Commented out direct call to add_readme.py
+2/-1     

@PRAgent4INC
Copy link
Collaborator

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The script directly modifies the input files in place without creating backups. This could lead to data loss if something goes wrong.

output_file = input_file
res = []
with open(input_file, "r") as f:
    for line in f.readlines():
        start = 0
        while True:
            pos = line.find('href="./', start)

            if pos>=0:
                # print("find", line)
                end_pos = line.find('">', pos)
                line = line[:end_pos]+'/README.md">'+line[end_pos+2:]
                start = end_pos+2
            else:
                break


        res.append(line)

with open(output_file, "w") as f:
    f.write("".join(res))
Code Comment

There is a commented-out line # output_file = f"{input_file}.new" that suggests an alternative approach to modifying files. Consider whether this approach is better for safety and clarity.

def main(inputs):
Script Invocation

The add_readme.py script is added but not invoked in the build.sh script. Ensure that the script is called correctly during the build process.

#python add_readme.py ./source/docs/source/examples/3.x_api/README.md

@PRAgent4INC
Copy link
Collaborator

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Enable script execution

Uncomment and adjust the script call to process the correct files.

docs/build_docs/build.sh [87]

-#python add_readme.py ./source/docs/source/examples/3.x_api/README.md
+python add_readme.py ./source/docs/source/examples/**/*.md
Suggestion importance[1-10]: 7

__

Why: Uncommenting and adjusting the script call to process the correct files is a relevant improvement. It ensures the script processes all necessary files, enhancing the build process.

Medium
Create file backup

Consider creating a backup of the original file before overwriting it.

docs/build_docs/add_readme.py [7]

-output_file = input_file
+output_file = f"{input_file}.new"
Suggestion importance[1-10]: 6

__

Why: Creating a backup of the original file before overwriting it is a good practice, but it's not a critical issue. It improves data safety but doesn't affect functionality.

Low
Correct link replacement

Ensure the replacement string is correctly formatted to avoid breaking links.

docs/build_docs/add_readme.py [18]

-line = line[:end_pos]+'/README.md">'+line[end_pos+2:]
+line = line[:end_pos] + '/README.md">' + line[end_pos + 2:]
Suggestion importance[1-10]: 3

__

Why: The existing code already correctly formats the replacement string. The suggested change does not alter the functionality and offers minimal improvement.

Low

@NeoZhangJianyu NeoZhangJianyu merged commit 49c6a89 into master Nov 6, 2025
11 checks passed
@NeoZhangJianyu NeoZhangJianyu deleted the jianyuzh_add_readme branch November 6, 2025 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants