Kczernik/papp 34959: added hook to generate readme in pre-commit #79
Conversation
…d-tools into kczernik/PAPP-34959
| @@ -9,3 +9,8 @@ | |||
| language: script | |||
| entry: pre-commit/package_app_dependencies | |||
| verbose: true | |||
| - id: readme_check | |||
There was a problem hiding this comment.
Why this place over an org-wide hooks? Just wondering where should we have this
There was a problem hiding this comment.
I think this is good, I think this is the same "level" as packaging dependencies
| app_json_name = [f for f in os.listdir(app_json_dir) | ||
| if f.endswith(".json") | ||
| and "postman_collection" not in f][0] | ||
| app_json_name = "" |
There was a problem hiding this comment.
Redundant? We either set this or exit below
There was a problem hiding this comment.
If you need to make sure it is set, you can always put the code after try:except into else: block.
| @@ -9,3 +9,8 @@ | |||
| language: script | |||
| entry: pre-commit/package_app_dependencies | |||
| verbose: true | |||
| - id: readme_check | |||
There was a problem hiding this comment.
I think this is good, I think this is the same "level" as packaging dependencies
|
|
||
| def validate_dir_path(path): | ||
| if not Path(path).is_dir(): | ||
| raise argparse.ArgumentTypeError(f"Provided value: {path} is not a directory.") |
There was a problem hiding this comment.
| raise argparse.ArgumentTypeError(f"Provided value: {path} is not a directory.") | |
| raise argparse.ArgumentTypeError(f"{path} is not a directory.") |
| import argparse | ||
| import sys | ||
| import logging | ||
| from build_docs import main |
There was a problem hiding this comment.
from build_docs import main as build_docs would be more readable imho
| @@ -9,3 +9,8 @@ | |||
| language: script | |||
| entry: pre-commit/package_app_dependencies | |||
| verbose: true | |||
| - id: readme_check | |||
There was a problem hiding this comment.
perhaps name readme-check would be more consistent
| try: | ||
| app_json_name = [f for f in os.listdir(app_json_dir) | ||
| if f.endswith(".json") | ||
| and "postman_collection" not in f][0] |
There was a problem hiding this comment.
I don't know where this is coming from, but... what if I have another json file in the app dir?
There was a problem hiding this comment.
There is a lot of pretty much this logic all over the place, I'd leave this as is but having a better way to tell which json is the json would be kewl
To make connector development more fluid we can add readme generation as a part of pre-commit, so that there will be less automatically generated commits, which makes git history more messy and hide latest test pipeline results.
PR adds new hook and handling lack of .json file
For now hook is in github-actions/start-release/readme_check.py.
Test on connector without .json file:
Test on connector with .json file:
