From 787def9816a765169f49b0f12810684662993481 Mon Sep 17 00:00:00 2001 From: Thomas Bui Date: Sat, 11 Dec 2021 19:53:25 -0800 Subject: [PATCH 1/7] Add check for strokes in check-bot --- .github/scripts/check_svgs_on_pr.py | 10 +++++----- .github/workflows/check_svgs_on_pr.yml | 5 +++-- .github/workflows/post_check_svgs_comment.yml | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.github/scripts/check_svgs_on_pr.py b/.github/scripts/check_svgs_on_pr.py index d6a725654..35729b5da 100644 --- a/.github/scripts/check_svgs_on_pr.py +++ b/.github/scripts/check_svgs_on_pr.py @@ -69,11 +69,11 @@ def check_svgs(svg_file_paths: List[Path]): if root.get("viewBox") != "0 0 128 128": err_msg.append("-'viewBox' is not '0 0 128 128' -> Set it or scale the file using https://www.iloveimg.com/resize-image/resize-svg.") - if root.get("x") is not None: - err_msg.append("-unneccessary 'x' attribute in svg root element -> Remove it") - - if root.get("y") is not None: - err_msg.append("-unneccessary 'y' attribute in svg root element -> Remove it") + # goes through all elems and check for strokes + for child in tree.iter(): + if child.get("stroke") != None: + err_msg.append("-SVG contains `stroke` property. This will get ignored by Icomoon. Please convert them to fills.") + break if len(err_msg) > 1: err_msgs.append("\n".join(err_msg)) diff --git a/.github/workflows/check_svgs_on_pr.yml b/.github/workflows/check_svgs_on_pr.yml index 70dd42446..a20329adc 100644 --- a/.github/workflows/check_svgs_on_pr.yml +++ b/.github/workflows/check_svgs_on_pr.yml @@ -19,6 +19,7 @@ jobs: with: token: ${{ secrets.GITHUB_TOKEN }} + # this is where we create the check_err_messages - name: Run the check_svg script run: > python ./.github/scripts/check_svgs_on_pr.py $HOME/files_added.json $HOME/files_modified.json @@ -27,8 +28,8 @@ jobs: uses: actions/upload-artifact@v2 if: success() with: - name: svg_err_messages - path: ./svg_err_messages.txt + name: check_err_messages + path: ./check_err_messages.txt - name: Save the pr num in an artifact shell: bash diff --git a/.github/workflows/post_check_svgs_comment.yml b/.github/workflows/post_check_svgs_comment.yml index 5ac89d3aa..5f5133d63 100644 --- a/.github/workflows/post_check_svgs_comment.yml +++ b/.github/workflows/post_check_svgs_comment.yml @@ -33,7 +33,7 @@ jobs: id: err_message_reader uses: juliangruber/read-file-action@v1.0.0 with: - path: ./svg_err_messages/svg_err_messages.txt + path: ./check_err_messages/check_err_messages.txt - name: Comment on the PR about the result - SVG Error uses: jungwinter/comment@v1 # let us comment on a specific PR From c959b04cc56ef7d654fdae32109e0c2707a9a581 Mon Sep 17 00:00:00 2001 From: Thomas Bui Date: Sun, 12 Dec 2021 11:58:53 -0800 Subject: [PATCH 2/7] Add check for svg file name --- .github/scripts/build_assets/util.py | 7 ++++++- .github/scripts/check_svgs_on_pr.py | 9 ++++++++- .github/workflows/post_check_svgs_comment.yml | 7 ++----- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/.github/scripts/build_assets/util.py b/.github/scripts/build_assets/util.py index 9ecbc8075..2a11f8e11 100644 --- a/.github/scripts/build_assets/util.py +++ b/.github/scripts/build_assets/util.py @@ -5,7 +5,6 @@ import sys import traceback - def exit_with_err(err: Exception): """ Exit the current step and display the err. @@ -66,3 +65,9 @@ def find_object_added_in_pr(icons: List[dict], pr_title: str): message = "util.find_object_added_in_pr: Couldn't find an icon matching the name in the PR title.\n" \ f"PR title is: '{pr_title}'" raise Exception(message) + + +valid_filename_pattern = re.compile(r"-(original|plain|line)(-wordmark)?\.svg$") +def is_svg_name_valid(filename: str): + return valid_filename_pattern.search(filename) is not None + diff --git a/.github/scripts/check_svgs_on_pr.py b/.github/scripts/check_svgs_on_pr.py index 35729b5da..9815ad9ce 100644 --- a/.github/scripts/check_svgs_on_pr.py +++ b/.github/scripts/check_svgs_on_pr.py @@ -2,6 +2,7 @@ from typing import List import xml.etree.ElementTree as et from pathlib import Path +import build_assets.util # pycharm complains that build_assets is an unresolved ref @@ -58,10 +59,16 @@ def check_svgs(svg_file_paths: List[Path]): err_msgs = [] for svg_path in svg_file_paths: try: + err_msg = [f"{svg_path}:"] + + # name check + if not is_svg_name_valid(svg_path.absolute()): + err_msg.append("-SVG file name didn't match our pattern of `name-(original|plain|line)(-wordmark)?.svg`") + + # svg check tree = et.parse(svg_path) root = tree.getroot() namespace = "{http://www.w3.org/2000/svg}" - err_msg = [f"{svg_path}:"] if root.tag != f"{namespace}svg": err_msg.append(f"-root is '{root.tag}'. Root must be an 'svg' element") diff --git a/.github/workflows/post_check_svgs_comment.yml b/.github/workflows/post_check_svgs_comment.yml index 5f5133d63..bbe415543 100644 --- a/.github/workflows/post_check_svgs_comment.yml +++ b/.github/workflows/post_check_svgs_comment.yml @@ -42,21 +42,18 @@ jobs: MESSAGE: | Hi! - I'm Devicons' SVG-Checker Bot and it seems we have some issues with your SVGs. + I'm the SVG-Checker Bot and we have some issues with your SVGs: - Here is what went wrong: ``` {0} ``` - For more reference on why these are errors, check out our [CONTRIBUTING guide](https://github.com/devicons/devicon/blob/develop/CONTRIBUTING.md#svgStandards) + Check our [CONTRIBUTING guide](https://github.com/devicons/devicon/blob/develop/CONTRIBUTING.md#svgStandards) for more details regarding these errors. Please address these issues. When you update this PR, I will check your SVGs again. Thanks for your help, SVG-Checker Bot :smile: - - PS. One day, I will be smart enough to fix these errors for you :persevere:. Until then, I can only point them out. with: type: create issue_number: ${{ steps.pr_num_reader.outputs.content }} From 5e357c9dd64db00b73175a703aae746038596276 Mon Sep 17 00:00:00 2001 From: Thomas Bui Date: Thu, 16 Dec 2021 12:25:08 -0800 Subject: [PATCH 3/7] Update gulpfile to remove x and y of svg elem --- gulpfile.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gulpfile.js b/gulpfile.js index 9a7c74344..f6cc2fae5 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -187,6 +187,12 @@ function configOptionCallback(file) { }, { removeDimensions: true // remove height and width + }, + { + name: "removeAttrs", + params: { + attrs: "svg:(x|y)" + } } ] }; From 6932e202ad5635b80027ca5778709cd157273f73 Mon Sep 17 00:00:00 2001 From: Thomas Bui Date: Sat, 18 Dec 2021 11:04:49 -0800 Subject: [PATCH 4/7] Revert name changes to err file --- .github/workflows/check_svgs_on_pr.yml | 4 ++-- .github/workflows/post_check_svgs_comment.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/check_svgs_on_pr.yml b/.github/workflows/check_svgs_on_pr.yml index a20329adc..ad93d9b00 100644 --- a/.github/workflows/check_svgs_on_pr.yml +++ b/.github/workflows/check_svgs_on_pr.yml @@ -28,8 +28,8 @@ jobs: uses: actions/upload-artifact@v2 if: success() with: - name: check_err_messages - path: ./check_err_messages.txt + name: svg_err_messages + path: ./svg_err_messages.txt - name: Save the pr num in an artifact shell: bash diff --git a/.github/workflows/post_check_svgs_comment.yml b/.github/workflows/post_check_svgs_comment.yml index bbe415543..40da0e352 100644 --- a/.github/workflows/post_check_svgs_comment.yml +++ b/.github/workflows/post_check_svgs_comment.yml @@ -33,7 +33,7 @@ jobs: id: err_message_reader uses: juliangruber/read-file-action@v1.0.0 with: - path: ./check_err_messages/check_err_messages.txt + path: ./svg_err_messages/svg_err_messages.txt - name: Comment on the PR about the result - SVG Error uses: jungwinter/comment@v1 # let us comment on a specific PR From 0c44d7f79e0a7bd5a73a4d363b66422bf31df822 Mon Sep 17 00:00:00 2001 From: Thomas Bui Date: Sun, 26 Dec 2021 15:49:13 -0800 Subject: [PATCH 5/7] check-bot now check devicon object as well --- .github/scripts/build_assets/arg_getters.py | 19 +++--- .github/scripts/build_assets/filehandler.py | 15 +++-- .github/scripts/build_assets/util.py | 4 +- .../{check_svgs_on_pr.py => check_icon_pr.py} | 66 +++++++++++++++++-- ...check_svgs_on_pr.yml => check_icon_pr.yml} | 15 ++--- .github/workflows/peek_icons.yml | 2 +- .github/workflows/post_check_svgs_comment.yml | 4 +- 7 files changed, 89 insertions(+), 36 deletions(-) rename .github/scripts/{check_svgs_on_pr.py => check_icon_pr.py} (57%) rename .github/workflows/{check_svgs_on_pr.yml => check_icon_pr.yml} (67%) diff --git a/.github/scripts/build_assets/arg_getters.py b/.github/scripts/build_assets/arg_getters.py index 80b88d3c2..f670ef142 100644 --- a/.github/scripts/build_assets/arg_getters.py +++ b/.github/scripts/build_assets/arg_getters.py @@ -34,12 +34,11 @@ def get_selenium_runner_args(peek_mode=False): action=PathResolverAction) if peek_mode: - parser.add_argument("--pr_title", + parser.add_argument("pr_title", help="The title of the PR that we are peeking at") else: parser.add_argument("token", - help="The GitHub token to access the GitHub REST API.", - type=str) + help="The GitHub token to access the GitHub REST API.") return parser.parse_args() @@ -49,13 +48,14 @@ def get_check_svgs_on_pr_args(): Get the commandline arguments for the check_svgs_on_pr.py. """ parser = ArgumentParser(description="Check the SVGs to ensure their attributes are correct. Run whenever a PR is opened") - parser.add_argument("files_added_json_path", - help="The path to the files_added.json created by the gh-action-get-changed-files@2.1.4", - action=PathResolverAction) - parser.add_argument("files_modified_json_path", - help="The path to the files_modified.json created by the gh-action-get-changed-files@2.1.4", + parser.add_argument("pr_title", + help="The title of the PR that we are peeking at") + + parser.add_argument("icons_folder_path", + help="The path to the icons folder", action=PathResolverAction) + return parser.parse_args() @@ -65,6 +65,5 @@ def get_release_message_args(): """ parser = ArgumentParser(description="Create a text containing the icons and features added since last release.") parser.add_argument("token", - help="The GitHub token to access the GitHub REST API.", - type=str) + help="The GitHub token to access the GitHub REST API.") return parser.parse_args() diff --git a/.github/scripts/build_assets/filehandler.py b/.github/scripts/build_assets/filehandler.py index 054431cd2..4c7d9acaa 100644 --- a/.github/scripts/build_assets/filehandler.py +++ b/.github/scripts/build_assets/filehandler.py @@ -207,6 +207,14 @@ def create_screenshot_folder(dir, screenshot_name: str="screenshots/"): finally: return str(screenshot_folder) +def write_to_file(path: str, value: any): + """ + Write the value to a file. + """ + with open(path, "w") as file: + file.write(value) + +# --- NOT USED CURRENTLY --- def get_added_modified_svgs(files_added_json_path: str, files_modified_json_path: str): """ @@ -231,10 +239,3 @@ def get_added_modified_svgs(files_added_json_path: str, return svgs - -def write_to_file(path: str, value: any): - """ - Write the value to a file. - """ - with open(path, "w") as file: - file.write(value) diff --git a/.github/scripts/build_assets/util.py b/.github/scripts/build_assets/util.py index 2a11f8e11..55ebb3219 100644 --- a/.github/scripts/build_assets/util.py +++ b/.github/scripts/build_assets/util.py @@ -67,7 +67,7 @@ def find_object_added_in_pr(icons: List[dict], pr_title: str): raise Exception(message) -valid_filename_pattern = re.compile(r"-(original|plain|line)(-wordmark)?\.svg$") +valid_svg_filename_pattern = re.compile(r"-(original|plain|line)(-wordmark)?\.svg$") def is_svg_name_valid(filename: str): - return valid_filename_pattern.search(filename) is not None + return valid_svg_filename_pattern.search(filename) is not None diff --git a/.github/scripts/check_svgs_on_pr.py b/.github/scripts/check_icon_pr.py similarity index 57% rename from .github/scripts/check_svgs_on_pr.py rename to .github/scripts/check_icon_pr.py index 9815ad9ce..763f238de 100644 --- a/.github/scripts/check_svgs_on_pr.py +++ b/.github/scripts/check_icon_pr.py @@ -2,7 +2,6 @@ from typing import List import xml.etree.ElementTree as et from pathlib import Path -import build_assets.util # pycharm complains that build_assets is an unresolved ref @@ -21,15 +20,20 @@ class SVG_STATUS_CODE(Enum): def main(): """ - Check the quality of the svgs. + Check the quality of the svgs IF this is an icon PR. Else, does nothing. If any svg error is found, create a json file called 'svg_err_messages.json' in the root folder that will contains the error messages. """ args = arg_getters.get_check_svgs_on_pr_args() try: + all_icons = filehandler.get_json_file_content(args.devicon_json_path) + + # get only the icon object that has the name matching the pr title + filtered_icon = util.find_object_added_in_pr(all_icons, args.pr_title) + check_devicon_object(filtered_icon) + # check the svgs - svgs = filehandler.get_added_modified_svgs(args.files_added_json_path, - args.files_modified_json_path) + svgs = filehandler.get_svgs_paths([filtered_icon], args.icons_folder_path) print("SVGs to check: ", *svgs, sep='\n') if len(svgs) == 0: @@ -41,9 +45,61 @@ def main(): filehandler.write_to_file("./svg_err_messages.txt", str(err_messages)) print("Task completed.") except Exception as e: + filehandler.write_to_file("./svg_err_messages.txt", str(e)) util.exit_with_err(e) +def check_devicon_object(icon: dict): + """ + Check that the devicon object added is up to standard. + :return a string containing the error messages if any. + """ + err_msgs = [] + try: + for tag in icon["tags"]: + if type(tag) != str: + raise TypeError() + except TypeError: + err_msgs.append("- 'tags' must be an array of strings, not: " + str(icon["tags"])) + except KeyError: + err_msgs.append("- missing key: 'tags'.") + + try: + if type(icon["versions"]) != dict: + err_msgs.append("- 'versions' must be an object.") + except KeyError: + err_msgs.append("- missing key: 'versions'.") + + try: + if type(icon["versions"]["svg"]) != list or len(icon["versions"]["svg"]) == 0: + err_msgs.append("- must contain at least 1 svg version in a list.") + except KeyError: + err_msgs.append("- missing key: 'svg' in 'versions'.") + + try: + if type(icon["versions"]["font"]) != list or len(icon["versions"]["svg"]) == 0: + err_msgs.append("- must contain at least 1 font version in a list.") + except KeyError: + err_msgs.append("- missing key: 'font' in 'versions'.") + + try: + if type(icon["color"]) != str or "#" not in icon["color"]: + err_msgs.append("- 'color' must be a string in the format '#abcdef'") + except KeyError: + err_msgs.append("- missing key: 'color'.") + + try: + if type(icon["aliases"]) != list: + err_msgs.append("- 'aliases' must be an array.") + except KeyError: + err_msgs.append("- missing key: 'aliases'.") + + if len(err_msgs) > 0: + message = "Error found in 'devicon.json' for '{}' entry: \n{}".format(icon["name"], "\n".join(err_msgs)) + raise ValueError(message) + return "" + + def check_svgs(svg_file_paths: List[Path]): """ Check the width, height, viewBox and style of each svgs passed in. @@ -62,7 +118,7 @@ def check_svgs(svg_file_paths: List[Path]): err_msg = [f"{svg_path}:"] # name check - if not is_svg_name_valid(svg_path.absolute()): + if not util.is_svg_name_valid(svg_path.name): err_msg.append("-SVG file name didn't match our pattern of `name-(original|plain|line)(-wordmark)?.svg`") # svg check diff --git a/.github/workflows/check_svgs_on_pr.yml b/.github/workflows/check_icon_pr.yml similarity index 67% rename from .github/workflows/check_svgs_on_pr.yml rename to .github/workflows/check_icon_pr.yml index ad93d9b00..ab97eb7f7 100644 --- a/.github/workflows/check_svgs_on_pr.yml +++ b/.github/workflows/check_icon_pr.yml @@ -1,9 +1,10 @@ -name: Check SVGs On PR +name: Check Icon PR on: pull_request jobs: check: - name: Check the SVGs' quality + name: Check the `devicon.json` and the SVGs' quality runs-on: ubuntu-18.04 + if: startsWith(github.event.pull_request.title, "new icon") # only checks icon PR steps: - uses: actions/checkout@v2 @@ -14,15 +15,11 @@ jobs: - name: Install dependencies run: python -m pip install --upgrade pip - - name: Get Changed Files and generate files_added.json & files_modified.json - uses: lots0logs/gh-action-get-changed-files@2.1.4 - with: - token: ${{ secrets.GITHUB_TOKEN }} - - # this is where we create the check_err_messages - name: Run the check_svg script + env: + PR_TITLE: ${{ github.event.pull_request.title }} run: > - python ./.github/scripts/check_svgs_on_pr.py $HOME/files_added.json $HOME/files_modified.json + python ./.github/scripts/check_icon_pr.py "$PR_TITLE" ./icons - name: Upload the err messages uses: actions/upload-artifact@v2 diff --git a/.github/workflows/peek_icons.yml b/.github/workflows/peek_icons.yml index 476c08df4..d0c1609ef 100644 --- a/.github/workflows/peek_icons.yml +++ b/.github/workflows/peek_icons.yml @@ -42,7 +42,7 @@ jobs: run: > python ./.github/scripts/icomoon_peek.py ./.github/scripts/build_assets/geckodriver-v0.29.1-win64/geckodriver.exe ./icomoon.json - ./devicon.json ./icons ./ --headless --pr_title "%PR_TITLE%" + ./devicon.json ./icons ./ "%PR_TITLE%" --headless - name: Upload the err messages (created by icomoon_peek.py) uses: actions/upload-artifact@v2 diff --git a/.github/workflows/post_check_svgs_comment.yml b/.github/workflows/post_check_svgs_comment.yml index 40da0e352..46780f1c6 100644 --- a/.github/workflows/post_check_svgs_comment.yml +++ b/.github/workflows/post_check_svgs_comment.yml @@ -1,7 +1,7 @@ name: Post the result of a SVG Check into its PR. on: workflow_run: - workflows: ['Check SVGs On PR'] + workflows: ['Check Icon PR'] types: - completed jobs: @@ -18,7 +18,7 @@ jobs: if: success() with: github_token: ${{ secrets.GITHUB_TOKEN }} - workflow: peek_icons.yml + workflow: check_icon_pr.yml run_id: ${{ github.event.workflow_run.id }} - name: Read the pr_num file From ebc871bc62ab475b6128899eb116c76d886941a0 Mon Sep 17 00:00:00 2001 From: Thomas Bui Date: Sun, 26 Dec 2021 16:20:12 -0800 Subject: [PATCH 6/7] Fixed minor bugs --- .github/scripts/build_assets/arg_getters.py | 8 ++++++-- .github/scripts/check_icon_pr.py | 15 +++++++-------- .github/scripts/icomoon_peek.py | 3 +-- .github/workflows/check_icon_pr.yml | 3 +-- .github/workflows/post_check_svgs_comment.yml | 2 +- 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/.github/scripts/build_assets/arg_getters.py b/.github/scripts/build_assets/arg_getters.py index f670ef142..dbc27c165 100644 --- a/.github/scripts/build_assets/arg_getters.py +++ b/.github/scripts/build_assets/arg_getters.py @@ -43,9 +43,9 @@ def get_selenium_runner_args(peek_mode=False): return parser.parse_args() -def get_check_svgs_on_pr_args(): +def get_check_icon_pr_args(): """ - Get the commandline arguments for the check_svgs_on_pr.py. + Get the commandline arguments for the check_icon_pr.py. """ parser = ArgumentParser(description="Check the SVGs to ensure their attributes are correct. Run whenever a PR is opened") @@ -56,6 +56,10 @@ def get_check_svgs_on_pr_args(): help="The path to the icons folder", action=PathResolverAction) + parser.add_argument("devicon_json_path", + help="The path to the devicon.json", + action=PathResolverAction) + return parser.parse_args() diff --git a/.github/scripts/check_icon_pr.py b/.github/scripts/check_icon_pr.py index 763f238de..ba3767acf 100644 --- a/.github/scripts/check_icon_pr.py +++ b/.github/scripts/check_icon_pr.py @@ -6,8 +6,7 @@ # pycharm complains that build_assets is an unresolved ref # don't worry about it, the script still runs -from build_assets import filehandler, arg_getters -from build_assets import util +from build_assets import filehandler, arg_getters, util class SVG_STATUS_CODE(Enum): @@ -24,13 +23,13 @@ def main(): If any svg error is found, create a json file called 'svg_err_messages.json' in the root folder that will contains the error messages. """ - args = arg_getters.get_check_svgs_on_pr_args() + args = arg_getters.get_check_icon_pr_args() try: all_icons = filehandler.get_json_file_content(args.devicon_json_path) # get only the icon object that has the name matching the pr title filtered_icon = util.find_object_added_in_pr(all_icons, args.pr_title) - check_devicon_object(filtered_icon) + devicon_err_msg = check_devicon_object(filtered_icon) # check the svgs svgs = filehandler.get_svgs_paths([filtered_icon], args.icons_folder_path) @@ -38,11 +37,11 @@ def main(): if len(svgs) == 0: print("No SVGs to check, ending script.") - err_messages = SVG_STATUS_CODE.NO_SVG.value + err_messages = str(SVG_STATUS_CODE.NO_SVG.value) else: err_messages = check_svgs(svgs) - filehandler.write_to_file("./svg_err_messages.txt", str(err_messages)) + filehandler.write_to_file("./svg_err_messages.txt", devicon_err_msg + "\n\n" + err_messages) print("Task completed.") except Exception as e: filehandler.write_to_file("./svg_err_messages.txt", str(e)) @@ -96,7 +95,7 @@ def check_devicon_object(icon: dict): if len(err_msgs) > 0: message = "Error found in 'devicon.json' for '{}' entry: \n{}".format(icon["name"], "\n".join(err_msgs)) - raise ValueError(message) + return message return "" @@ -145,7 +144,7 @@ def check_svgs(svg_file_paths: List[Path]): if len(err_msgs) > 0: return "\n\n".join(err_msgs) - return SVG_STATUS_CODE.SVG_OK.value + return str(SVG_STATUS_CODE.SVG_OK.value) if __name__ == "__main__": diff --git a/.github/scripts/icomoon_peek.py b/.github/scripts/icomoon_peek.py index 2dcc7143c..4c6e8ea5b 100644 --- a/.github/scripts/icomoon_peek.py +++ b/.github/scripts/icomoon_peek.py @@ -1,6 +1,5 @@ from build_assets.selenium_runner.PeekSeleniumRunner import PeekSeleniumRunner -from build_assets import filehandler, arg_getters -from build_assets import util +from build_assets import filehandler, arg_getters, util def main(): diff --git a/.github/workflows/check_icon_pr.yml b/.github/workflows/check_icon_pr.yml index ab97eb7f7..eb8600c0c 100644 --- a/.github/workflows/check_icon_pr.yml +++ b/.github/workflows/check_icon_pr.yml @@ -18,8 +18,7 @@ jobs: - name: Run the check_svg script env: PR_TITLE: ${{ github.event.pull_request.title }} - run: > - python ./.github/scripts/check_icon_pr.py "$PR_TITLE" ./icons + run: python ./.github/scripts/check_icon_pr.py "$PR_TITLE" ./icons ./devicon.json - name: Upload the err messages uses: actions/upload-artifact@v2 diff --git a/.github/workflows/post_check_svgs_comment.yml b/.github/workflows/post_check_svgs_comment.yml index 46780f1c6..a4a635bac 100644 --- a/.github/workflows/post_check_svgs_comment.yml +++ b/.github/workflows/post_check_svgs_comment.yml @@ -42,7 +42,7 @@ jobs: MESSAGE: | Hi! - I'm the SVG-Checker Bot and we have some issues with your SVGs: + I'm the `check-bot` and we have some issues with your PR: ``` {0} From 7515d87d360ff341564cf90d26a8bba10a796763 Mon Sep 17 00:00:00 2001 From: Thomas Bui Date: Mon, 27 Dec 2021 12:12:04 -0800 Subject: [PATCH 7/7] Improve logging in check_icon --- .github/scripts/build_assets/filehandler.py | 7 +- .github/scripts/check_icon_pr.py | 67 ++++++++++++------- .github/scripts/icomoon_peek.py | 58 +--------------- .github/workflows/check_icon_pr.yml | 9 +-- ...ent.yml => post_check_icon_pr_comment.yml} | 6 +- 5 files changed, 52 insertions(+), 95 deletions(-) rename .github/workflows/{post_check_svgs_comment.yml => post_check_icon_pr_comment.yml} (94%) diff --git a/.github/scripts/build_assets/filehandler.py b/.github/scripts/build_assets/filehandler.py index 4c7d9acaa..29e095928 100644 --- a/.github/scripts/build_assets/filehandler.py +++ b/.github/scripts/build_assets/filehandler.py @@ -68,7 +68,7 @@ def get_svgs_paths(new_icons: List[dict], icons_folder_path: str, folder_path = Path(icons_folder_path, icon_info['name']) if not folder_path.is_dir(): - raise ValueError(f"Invalid path. This is not a directory: {folder_path}.") + raise ValueError(f"Invalid path. This is not a directory: '{folder_path}'.") if icon_versions_only: get_icon_svgs_paths(folder_path, icon_info, file_paths, as_str) @@ -100,7 +100,7 @@ def get_icon_svgs_paths(folder_path: Path, icon_info: dict, if path.exists(): file_paths.append(str(path) if as_str else path) else: - raise ValueError(f"This path doesn't exist: {path}") + raise ValueError(f"This path doesn't exist: '{path}'") def get_all_svgs_paths(folder_path: Path, icon_info: dict, @@ -119,7 +119,7 @@ def get_all_svgs_paths(folder_path: Path, icon_info: dict, if path.exists(): file_paths.append(str(path) if as_str else path) else: - raise ValueError(f"This path doesn't exist: {path}") + raise ValueError(f"This path doesn't exist: '{path}'") def is_alias(font_version: str, aliases: List[dict]): @@ -238,4 +238,3 @@ def get_added_modified_svgs(files_added_json_path: str, svgs.append(path) return svgs - diff --git a/.github/scripts/check_icon_pr.py b/.github/scripts/check_icon_pr.py index ba3767acf..5116986fb 100644 --- a/.github/scripts/check_icon_pr.py +++ b/.github/scripts/check_icon_pr.py @@ -1,4 +1,3 @@ -from enum import Enum from typing import List import xml.etree.ElementTree as et from pathlib import Path @@ -9,14 +8,6 @@ from build_assets import filehandler, arg_getters, util -class SVG_STATUS_CODE(Enum): - """ - The status codes to check for in post_check_svgs_comment.yml - """ - NO_SVG = 0 # action: do nothing - SVG_OK = 1 # action: let user know their svgs are fine - - def main(): """ Check the quality of the svgs IF this is an icon PR. Else, does nothing. @@ -29,22 +20,39 @@ def main(): # get only the icon object that has the name matching the pr title filtered_icon = util.find_object_added_in_pr(all_icons, args.pr_title) + print("Checking devicon.json object: " + str(filtered_icon)) devicon_err_msg = check_devicon_object(filtered_icon) - # check the svgs - svgs = filehandler.get_svgs_paths([filtered_icon], args.icons_folder_path) - print("SVGs to check: ", *svgs, sep='\n') + # check the file names + filename_err_msg = "" + svgs = None + try: + svgs = filehandler.get_svgs_paths([filtered_icon], args.icons_folder_path, as_str=False) + print("SVGs to check: ", *svgs, sep='\n') + except ValueError as e: + filename_err_msg = "Error found regarding filenames:\n- " + e.args[0] - if len(svgs) == 0: + # check the svgs + if svgs is None or len(svgs) == 0: print("No SVGs to check, ending script.") - err_messages = str(SVG_STATUS_CODE.NO_SVG.value) + svg_err_msg = "Error checking SVGs: no SVGs to check. Might be caused by above issues." else: - err_messages = check_svgs(svgs) + svg_err_msg = check_svgs(svgs) - filehandler.write_to_file("./svg_err_messages.txt", devicon_err_msg + "\n\n" + err_messages) + err_msg = [] + if devicon_err_msg != "": + err_msg.append(devicon_err_msg) + + if filename_err_msg != "": + err_msg.append(filename_err_msg) + + if svg_err_msg != "": + err_msg.append(svg_err_msg) + + filehandler.write_to_file("./err_messages.txt", "\n\n".join(err_msg)) print("Task completed.") except Exception as e: - filehandler.write_to_file("./svg_err_messages.txt", str(e)) + filehandler.write_to_file("./err_messages.txt", str(e)) util.exit_with_err(e) @@ -72,12 +80,20 @@ def check_devicon_object(icon: dict): try: if type(icon["versions"]["svg"]) != list or len(icon["versions"]["svg"]) == 0: err_msgs.append("- must contain at least 1 svg version in a list.") + + for version in icon["versions"]["svg"]: + if not util.is_version_name_valid(version): + err_msgs.append(f"- Invalid version name in versions['svg']: '{version}'. Must match regexp: (original|plain|line)(-wordmark)?") except KeyError: err_msgs.append("- missing key: 'svg' in 'versions'.") try: if type(icon["versions"]["font"]) != list or len(icon["versions"]["svg"]) == 0: err_msgs.append("- must contain at least 1 font version in a list.") + + for version in icon["versions"]["font"]: + if not util.is_version_name_valid(version): + err_msgs.append(f"- Invalid version name in versions['font']: '{version}'. Must match regexp: (original|plain|line)(-wordmark)?") except KeyError: err_msgs.append("- missing key: 'font' in 'versions'.") @@ -96,15 +112,14 @@ def check_devicon_object(icon: dict): if len(err_msgs) > 0: message = "Error found in 'devicon.json' for '{}' entry: \n{}".format(icon["name"], "\n".join(err_msgs)) return message - return "" + return "" def check_svgs(svg_file_paths: List[Path]): """ Check the width, height, viewBox and style of each svgs passed in. The viewBox must be '0 0 128 128'. - If the svg has a width and height attr, ensure it's '128px'. - The style must not contain any 'fill' declarations. + The style must not contain any 'stroke' declarations. If any error is found, they will be thrown. :param: svg_file_paths, the file paths to the svg to check for. :return: None if there no errors. If there is, return a JSON.stringified @@ -114,11 +129,11 @@ def check_svgs(svg_file_paths: List[Path]): err_msgs = [] for svg_path in svg_file_paths: try: - err_msg = [f"{svg_path}:"] + err_msg = [f"SVG Error in '{svg_path.name}':"] # name check if not util.is_svg_name_valid(svg_path.name): - err_msg.append("-SVG file name didn't match our pattern of `name-(original|plain|line)(-wordmark)?.svg`") + err_msg.append("- SVG file name didn't match our pattern of `name-(original|plain|line)(-wordmark)?.svg`") # svg check tree = et.parse(svg_path) @@ -126,15 +141,15 @@ def check_svgs(svg_file_paths: List[Path]): namespace = "{http://www.w3.org/2000/svg}" if root.tag != f"{namespace}svg": - err_msg.append(f"-root is '{root.tag}'. Root must be an 'svg' element") + err_msg.append(f"- root is '{root.tag}'. Root must be an 'svg' element") if root.get("viewBox") != "0 0 128 128": - err_msg.append("-'viewBox' is not '0 0 128 128' -> Set it or scale the file using https://www.iloveimg.com/resize-image/resize-svg.") + err_msg.append("- 'viewBox' is not '0 0 128 128' -> Set it or scale the file using https://www.iloveimg.com/resize-image/resize-svg.") # goes through all elems and check for strokes for child in tree.iter(): if child.get("stroke") != None: - err_msg.append("-SVG contains `stroke` property. This will get ignored by Icomoon. Please convert them to fills.") + err_msg.append("- SVG contains `stroke` property. This will get ignored by Icomoon. Please convert them to fills.") break if len(err_msg) > 1: @@ -144,7 +159,7 @@ def check_svgs(svg_file_paths: List[Path]): if len(err_msgs) > 0: return "\n\n".join(err_msgs) - return str(SVG_STATUS_CODE.SVG_OK.value) + return "" if __name__ == "__main__": diff --git a/.github/scripts/icomoon_peek.py b/.github/scripts/icomoon_peek.py index fe53ad482..80a6a6ca3 100644 --- a/.github/scripts/icomoon_peek.py +++ b/.github/scripts/icomoon_peek.py @@ -10,12 +10,10 @@ def main(): # get only the icon object that has the name matching the pr title filtered_icon = util.find_object_added_in_pr(all_icons, args.pr_title) - check_devicon_object(filtered_icon) - print("Icon being checked:", filtered_icon, sep = "\n", end='\n\n') - - runner = PeekSeleniumRunner(args.download_path, args.geckodriver_path, args.headless) svgs = filehandler.get_svgs_paths([filtered_icon], args.icons_folder_path, True) screenshot_folder = filehandler.create_screenshot_folder("./") + + runner = PeekSeleniumRunner(args.download_path, args.geckodriver_path, args.headless) svgs_with_strokes = runner.peek(svgs, screenshot_folder, filtered_icon) print("Task completed.") @@ -32,57 +30,5 @@ def main(): runner.close() -def check_devicon_object(icon: dict): - """ - Check that the devicon object added is up to standard. - :param icon: a dictionary containing info on an icon. Taken from the devicon.json. - :return a string containing the error messages if any. - """ - err_msgs = [] - try: - for tag in icon["tags"]: - if type(tag) != str: - raise TypeError() - except TypeError: - err_msgs.append("- 'tags' must be an array of strings, not: " + str(icon["tags"])) - except KeyError: - err_msgs.append("- missing key: 'tags'.") - - try: - if type(icon["versions"]) != dict: - err_msgs.append("- 'versions' must be an object.") - except KeyError: - err_msgs.append("- missing key: 'versions'.") - - try: - if type(icon["versions"]["svg"]) != list or len(icon["versions"]["svg"]) == 0: - err_msgs.append("- must contain at least 1 svg version in a list.") - except KeyError: - err_msgs.append("- missing key: 'svg' in 'versions'.") - - try: - if type(icon["versions"]["font"]) != list or len(icon["versions"]["svg"]) == 0: - err_msgs.append("- must contain at least 1 font version in a list.") - except KeyError: - err_msgs.append("- missing key: 'font' in 'versions'.") - - try: - if type(icon["color"]) != str or "#" not in icon["color"]: - err_msgs.append("- 'color' must be a string in the format '#abcdef'") - except KeyError: - err_msgs.append("- missing key: 'color'.") - - try: - if type(icon["aliases"]) != list: - err_msgs.append("- 'aliases' must be an array.") - except KeyError: - err_msgs.append("- missing key: 'aliases'.") - - if len(err_msgs) > 0: - message = "Error found in 'devicon.json' for '{}' entry: \n{}".format(icon["name"], "\n".join(err_msgs)) - raise ValueError(message) - return "" - - if __name__ == "__main__": main() diff --git a/.github/workflows/check_icon_pr.yml b/.github/workflows/check_icon_pr.yml index eb8600c0c..1247ed8ae 100644 --- a/.github/workflows/check_icon_pr.yml +++ b/.github/workflows/check_icon_pr.yml @@ -4,7 +4,7 @@ jobs: check: name: Check the `devicon.json` and the SVGs' quality runs-on: ubuntu-18.04 - if: startsWith(github.event.pull_request.title, "new icon") # only checks icon PR + if: startsWith(github.event.pull_request.title, 'new icon') # only checks icon PR steps: - uses: actions/checkout@v2 @@ -12,9 +12,6 @@ jobs: with: python-version: 3.8 - - name: Install dependencies - run: python -m pip install --upgrade pip - - name: Run the check_svg script env: PR_TITLE: ${{ github.event.pull_request.title }} @@ -24,8 +21,8 @@ jobs: uses: actions/upload-artifact@v2 if: success() with: - name: svg_err_messages - path: ./svg_err_messages.txt + name: err_messages + path: ./err_messages.txt - name: Save the pr num in an artifact shell: bash diff --git a/.github/workflows/post_check_svgs_comment.yml b/.github/workflows/post_check_icon_pr_comment.yml similarity index 94% rename from .github/workflows/post_check_svgs_comment.yml rename to .github/workflows/post_check_icon_pr_comment.yml index a4a635bac..716ca752b 100644 --- a/.github/workflows/post_check_svgs_comment.yml +++ b/.github/workflows/post_check_icon_pr_comment.yml @@ -1,4 +1,4 @@ -name: Post the result of a SVG Check into its PR. +name: Post the result of the check_icon_pr workflow into its PR. on: workflow_run: workflows: ['Check Icon PR'] @@ -33,11 +33,11 @@ jobs: id: err_message_reader uses: juliangruber/read-file-action@v1.0.0 with: - path: ./svg_err_messages/svg_err_messages.txt + path: ./err_messages/err_messages.txt - name: Comment on the PR about the result - SVG Error uses: jungwinter/comment@v1 # let us comment on a specific PR - if: success() && (steps.err_message_reader.outputs.content != '0' && steps.err_message_reader.outputs.content != '1') + if: success() && (steps.err_message_reader.outputs.content != '') env: MESSAGE: | Hi!