New script to generate the command docs #4664
Conversation
| def get_current_branch(): | ||
| """Returns the current Git branch name.""" | ||
| result = subprocess.run(["git", "rev-parse", "--abbrev-ref", "HEAD"], capture_output=True, text=True) | ||
| return result.stdout.strip() | ||
|
|
||
|
|
||
| def get_modified_files(): | ||
| """Returns a list of files modified in the current commit.""" | ||
| result = subprocess.run(["git", "diff", "--cached", "--name-only"], capture_output=True, text=True) | ||
| return result.stdout.splitlines() | ||
|
|
||
|
|
||
| def extract_changed_commands(modified_files): | ||
| """Extract command names from modified _setup.py files.""" | ||
| changed_commands = [] | ||
| for file in modified_files: | ||
| if file.endswith("_setup.py"): | ||
| command_name = Path(file).stem.replace("_setup", "") | ||
| changed_commands.append(command_name) | ||
| return changed_commands |
There was a problem hiding this comment.
Add return argument and return value docstring as well as a return type: (-> str and so on)
There was a problem hiding this comment.
True for all functions
| # Check if the branch should be excluded | ||
| current_branch = get_current_branch() | ||
| if re.match(EXCLUDED_BRANCHES_REGEX, current_branch): | ||
| print(f"Pre-commit hook skipped on branch '{current_branch}'") |
There was a problem hiding this comment.
| print(f"Pre-commit hook skipped on branch '{current_branch}'") | |
| print(f"Generate docs pre-commit hook skipped on branch '{current_branch}'") |
| if not changed_commands: | ||
| print("No modified _setup.py files found. Skipping documentation generation.") | ||
| sys.exit(0) |
There was a problem hiding this comment.
How does it make sense tho? Per the hook config (files: ^.*_setup\.py$) the hook only runs when the setup file change no?
|
|
||
| # Run the documentation generation script with all changed commands | ||
| print(f"Generating documentation for modified commands: {changed_commands}") | ||
| subprocess.run([sys.executable, "generate_docs.py", *changed_commands]) |
There was a problem hiding this comment.
Why are we using a subprocess here? why not just import and run the relevant python module?
|
|
||
| # Receive the list of modified files from command-line arguments | ||
| modified_files = sys.argv[1:] | ||
| changed_commands = extract_changed_commands(modified_files) |
There was a problem hiding this comment.
I'm confused, you called this script with changed commands (not changed files) - subprocess.run([sys.executable, "generate_docs.py", *changed_commands])
Why are we trying to re-extract the command names?
| options_text += ( | ||
| f"- **{param_name}**: {param.help or 'No description provided'}\n" | ||
| ) | ||
| if param.default is not None: |
There was a problem hiding this comment.
why is not None:? Isn't if param.default enough? if for example it is'' you still don't want the condition to apply right?
|
|
||
| def get_command_description(command_name: str) -> str: | ||
| """Retrieve the description (docstring) for the command.""" | ||
| command = get_sdk_command(command_name) |
There was a problem hiding this comment.
What if the response is "No README found for command..."?
|
|
||
| def generate_docs_for_command(command_name: str): | ||
| """Generate documentation for a specific command.""" | ||
| description = get_command_description(command_name) |
There was a problem hiding this comment.
Are we ok with the description being 'No description provided'? Where is it going be added to?
| command_doc_path = Path("demisto_sdk/commands") / command_name / "README.md" | ||
|
|
||
| if not command_doc_path.exists(): | ||
| print(f"README.md not found for command: {command_name}") # noqa: T201 |
There was a problem hiding this comment.
Why not generate a new one?
| """Generate documentation for a specific command.""" | ||
| description = get_command_description(command_name) | ||
| options = get_command_options(command_name) | ||
| update_readme(command_name, description, options) |
There was a problem hiding this comment.
Is it enough to write the file? what about committing etc?
Related Issues
fixes: https://jira-dc.paloaltonetworks.com/browse/CIAC-12077
https://jira-dc.paloaltonetworks.com/browse/CIAC-12078
Description