ci: add github action that analyzes all python functions #5516
Conversation
This new github action runs a check on each the source branch and the target branch and finds all functions that are missing docstrings or functions that are missing type hints. And by checking if the number of either of those two has increased in the source branch, then it knows that new code was added that is missing either a docstring or type hints.
|
|
||
| - name: Analyze target branch | ||
| run: | | ||
| python tools/analyze_functions.py analyze target_functions_info.yml cloudinit tests |
There was a problem hiding this comment.
maybe remove tests dir??
There was a problem hiding this comment.
todo: add ability to specify dirs per docstrings and type hints check
| with: | ||
| ref: ${{ github.base_ref }} | ||
|
|
||
| - name: Download source analysis result |
There was a problem hiding this comment.
add comments for why this is needed
| @@ -0,0 +1,50 @@ | |||
| # .github/workflows/analyze_functions.yml | |||
| name: Analyze Python Functions | |||
There was a problem hiding this comment.
james: rename request from something vague to better describe what this workflow is doing on CI reports
| @@ -0,0 +1,134 @@ | |||
| import ast | |||
There was a problem hiding this comment.
james: add a docstr here to better describe the intent of this module and how/why to run it.
There was a problem hiding this comment.
Synchronous review done w/ @TheRealFalcon @blackboxsw @aciba90 @catmsred
| @@ -0,0 +1,134 @@ | |||
| import ast | |||
There was a problem hiding this comment.
add detailed docstring about what it does, and how it is used, give some examples
| print(f"Total functions without type hints: {count_functions(functions_info, 'functions_without_type_hints')}") | ||
| print(f"Total functions without docstrings: {count_functions(functions_info, 'functions_without_docstrings')}") | ||
|
|
||
| def compare(source_file, target_file): |
|
|
||
| def has_docstring(node): | ||
| """Check if a node has a docstring.""" | ||
| return isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and isinstance(node.body[0], ast.Expr) and isinstance(node.body[0].value, ast.Str) |
There was a problem hiding this comment.
refactor this to have local variables with descriptive names!!
| return isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and isinstance(node.body[0], ast.Expr) and isinstance(node.body[0].value, ast.Str) | ||
|
|
||
| def find_functions_info(directories): | ||
| total_number_of_functions = 0 |
There was a problem hiding this comment.
move this below docstring
| import argparse | ||
| import sys | ||
|
|
||
| def has_type_hint(node): |
There was a problem hiding this comment.
add type hints to all functions plz
| run: | | ||
| python tools/analyze_functions.py analyze target_functions_info.yml cloudinit tests | ||
|
|
||
| - name: Compare analysis results |
There was a problem hiding this comment.
break this into two different steps - one for comparing docstrings and one for type hints. this will help with readability and visibility of github action
| target_functions_without_docstrings = set(target_info.get(file, {}).get('functions_without_docstrings', [])) | ||
| new_docstrings = source_functions_without_docstrings - target_functions_without_docstrings | ||
| if new_docstrings: | ||
| new_functions_without_docstrings.extend([f"{file}: {func}" for func in new_docstrings]) |
There was a problem hiding this comment.
mayhaps break this duplicate code (lines 84-94) into a helper function
| parser = argparse.ArgumentParser(description='Analyze Python functions in directories or compare results.') | ||
| subparsers = parser.add_subparsers(dest='command') | ||
|
|
||
| analyze_parser = subparsers.add_parser('analyze', help='Analyze the functions in directories.') |
There was a problem hiding this comment.
split this into two different commands: one for docstrings and one for type hints. this will then make it so that two yml files are generated, one for each. so changes will need to be made to github action as well.
|
Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close. If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon. (If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.) |
github-actions
bot
added
the
stale-pr
This new github action runs a check on each the source branch and the target branch and finds all functions that are missing docstrings or functions that are missing type hints. And by checking if the number of either of those two has increased in the source branch, then it knows that new code was added that is missing either a docstring or type hints.
Proposed Commit Message
Additional Context
Test Steps
Merge type