From b944e35c25c031c8c897479016fd7d0f26cf7461 Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Thu, 4 Apr 2024 13:19:32 +0200 Subject: [PATCH 01/17] Update MaterialNode initialization and metadata retrieval Although this is rare scenario.This allows for metadata retrieval from the container directly if one is provided, otherwise metadata is fetched using the container_id from the ContainerRegistry. Updated the MaterialNode creation in the VariantNode class(only one scenario) to pass the container. CURA-11748 --- cura/Machines/MaterialNode.py | 19 +++++++++++++------ cura/Machines/VariantNode.py | 2 +- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/cura/Machines/MaterialNode.py b/cura/Machines/MaterialNode.py index 179451ff67..a833ad0470 100644 --- a/cura/Machines/MaterialNode.py +++ b/cura/Machines/MaterialNode.py @@ -21,18 +21,25 @@ class MaterialNode(ContainerNode): Its subcontainers are quality profiles. """ - def __init__(self, container_id: str, variant: "VariantNode") -> None: + def __init__(self, container_id: str, variant: "VariantNode", *, container: ContainerInterface = None) -> None: super().__init__(container_id) self.variant = variant self.qualities = {} # type: Dict[str, QualityNode] # Mapping container IDs to quality profiles. self.materialChanged = Signal() # Triggered when the material is removed or its metadata is updated. container_registry = ContainerRegistry.getInstance() - my_metadata = container_registry.findContainersMetadata(id = container_id)[0] - self.base_file = my_metadata["base_file"] - self.material_type = my_metadata["material"] - self.brand = my_metadata["brand"] - self.guid = my_metadata["GUID"] + + if container is not None: + self.base_file = container.getMetaDataEntry("base_file") + self.material_type = container.getMetaDataEntry("material") + self.brand = container.getMetaDataEntry("brand") + self.guid = container.getMetaDataEntry("GUID") + else: + my_metadata = container_registry.findContainersMetadata(id = container_id)[0] + self.base_file = my_metadata["base_file"] + self.material_type = my_metadata["material"] + self.brand = my_metadata["brand"] + self.guid = my_metadata["GUID"] self._loadAll() container_registry.containerRemoved.connect(self._onRemoved) container_registry.containerMetaDataChanged.connect(self._onMetadataChanged) diff --git a/cura/Machines/VariantNode.py b/cura/Machines/VariantNode.py index 6c3e9359d0..b976841aa7 100644 --- a/cura/Machines/VariantNode.py +++ b/cura/Machines/VariantNode.py @@ -148,7 +148,7 @@ class VariantNode(ContainerNode): if "empty_material" in self.materials: del self.materials["empty_material"] - self.materials[base_file] = MaterialNode(container.getId(), variant = self) + self.materials[base_file] = MaterialNode(container.getId(), variant = self, container = container) self.materials[base_file].materialChanged.connect(self.materialsChanged) self.materialsChanged.emit(self.materials[base_file]) From e5fb40b48c7ff1e86daec930fab718e3199959dc Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Fri, 5 Apr 2024 10:02:08 +0200 Subject: [PATCH 02/17] Add material temperature check in linter CURA-10904 --- .printer-linter | 1 + .../src/printerlinter/linters/defintion.py | 44 ++++++++++++++++--- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/.printer-linter b/.printer-linter index 3a42a5c033..e5472a0502 100644 --- a/.printer-linter +++ b/.printer-linter @@ -3,6 +3,7 @@ checks: diagnostic-mesh-file-size: true diagnostic-definition-redundant-override: true diagnostic-resources-macos-app-directory-name: true + diagnostic-material-temperature-defined: true fixes: diagnostic-definition-redundant-override: true format: diff --git a/printer-linter/src/printerlinter/linters/defintion.py b/printer-linter/src/printerlinter/linters/defintion.py index c4e955a2a5..6dfc5fcd44 100644 --- a/printer-linter/src/printerlinter/linters/defintion.py +++ b/printer-linter/src/printerlinter/linters/defintion.py @@ -28,6 +28,10 @@ class Definition(Linter): for check in self.checkRedefineOverride(): yield check + if self._settings["checks"].get("diagnostic-material-temperature-defined", False): + for check in self.checkMaterialTemperature(): + yield check + # Add other which will yield Diagnostic's # TODO: A check to determine if the user set value is with the min and max value defined in the parent and doesn't trigger a warning # TODO: A check if the key exist in the first place @@ -41,7 +45,7 @@ class Definition(Linter): definition = self._definitions[definition_name] if "overrides" in definition and definition_name not in ("fdmprinter", "fdmextruder"): for key, value_dict in definition["overrides"].items(): - is_redefined, child_key, child_value, parent = self._isDefinedInParent(key, value_dict, definition['inherits']) + is_redefined, child_key, child_value, parent, inherited_by= self._isDefinedInParent(key, value_dict, definition['inherits']) if is_redefined: redefined = re.compile(r'.*(\"' + key + r'\"[\s\:\S]*?)\{[\s\S]*?\},?') found = redefined.search(self._content) @@ -59,12 +63,40 @@ class Definition(Linter): yield Diagnostic( file = self._file, diagnostic_name = "diagnostic-definition-redundant-override", - message = f"Overriding {key} with the same value ({child_key}: {child_value}) as defined in parent definition: {definition['inherits']}", + message = f"Overriding {key} with the same value ({child_key}: {child_value}) as defined in parent definition: {inherited_by}", level = "Warning", offset = found.span(0)[0], replacements = replacements ) + def checkMaterialTemperature(self) -> Iterator[Diagnostic]: + """Checks if definition file has material tremperature defined within them""" + definition_name = list(self._definitions.keys())[0] + definition = self._definitions[definition_name] + if "overrides" in definition and definition_name not in ("fdmprinter", "fdmextruder"): + for key, value_dict in definition["overrides"].items(): + if "temperature" in key and "material" in key: + + redefined = re.compile(r'.*(\"' + key + r'\"[\s\:\S]*?)\{[\s\S]*?\},?') + found = redefined.search(self._content) + if len(found.group().splitlines()) > 1: + replacements = [] + else: + replacements = [Replacement( + file=self._file, + offset=found.span(1)[0], + length=len(found.group()), + replacement_text="")] + + yield Diagnostic( + file=self._file, + diagnostic_name="diagnostic-definition-redundant-override", + message=f"Overriding {key} as it belongs to material temperature catagory and shouldn't be placed in machine definitions", + level="Warning", + offset=found.span(0)[0], + replacements=replacements + ) + def _loadDefinitionFiles(self, definition_file) -> None: """ Loads definition file contents into self._definitions. Also load parent definition if it exists. """ definition_name = Path(definition_file.stem).stem @@ -85,8 +117,8 @@ class Definition(Linter): def _isDefinedInParent(self, key, value_dict, inherits_from): if self._ignore(key, "diagnostic-definition-redundant-override"): - return False, None, None, None - if "overrides" not in self._definitions[inherits_from]: + return False, None, None, None, None + if key not in self._definitions[inherits_from]["overrides"]: return self._isDefinedInParent(key, value_dict, self._definitions[inherits_from]["inherits"]) parent = self._definitions[inherits_from]["overrides"] @@ -114,11 +146,11 @@ class Definition(Linter): v = child_value cv = check_value if v == cv: - return True, child_key, child_value, parent + return True, child_key, child_value, parent, inherits_from if "inherits" in parent: return self._isDefinedInParent(key, value_dict, parent["inherits"]) - return False, None, None, None + return False, None, None, None, None def _loadBasePrinterSettings(self): settings = {} From 66097413233957881e0abe40256a97b45f8ae97b Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Fri, 5 Apr 2024 10:06:28 +0200 Subject: [PATCH 03/17] Fix potential key error in printer linter CURA-10904 --- printer-linter/src/printerlinter/linters/defintion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/printer-linter/src/printerlinter/linters/defintion.py b/printer-linter/src/printerlinter/linters/defintion.py index 6dfc5fcd44..c892a07e02 100644 --- a/printer-linter/src/printerlinter/linters/defintion.py +++ b/printer-linter/src/printerlinter/linters/defintion.py @@ -118,7 +118,7 @@ class Definition(Linter): def _isDefinedInParent(self, key, value_dict, inherits_from): if self._ignore(key, "diagnostic-definition-redundant-override"): return False, None, None, None, None - if key not in self._definitions[inherits_from]["overrides"]: + if "overrides" not in self._definitions[inherits_from] or key not in self._definitions[inherits_from]["overrides"]: return self._isDefinedInParent(key, value_dict, self._definitions[inherits_from]["inherits"]) parent = self._definitions[inherits_from]["overrides"] From 794cdfd077d2d575b4404c16aadc0a4a3c657b70 Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Fri, 5 Apr 2024 11:59:39 +0200 Subject: [PATCH 04/17] Same as before. CURA-10904 --- printer-linter/src/printerlinter/linters/defintion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/printer-linter/src/printerlinter/linters/defintion.py b/printer-linter/src/printerlinter/linters/defintion.py index c892a07e02..bf9a39452a 100644 --- a/printer-linter/src/printerlinter/linters/defintion.py +++ b/printer-linter/src/printerlinter/linters/defintion.py @@ -118,7 +118,7 @@ class Definition(Linter): def _isDefinedInParent(self, key, value_dict, inherits_from): if self._ignore(key, "diagnostic-definition-redundant-override"): return False, None, None, None, None - if "overrides" not in self._definitions[inherits_from] or key not in self._definitions[inherits_from]["overrides"]: + if "overrides" not in self._definitions[inherits_from]: return self._isDefinedInParent(key, value_dict, self._definitions[inherits_from]["inherits"]) parent = self._definitions[inherits_from]["overrides"] From 38382eeec749df4f71c3d801ed57263560ee5c7b Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Fri, 5 Apr 2024 15:55:30 +0200 Subject: [PATCH 05/17] Add detection for deleted files in printer linter This update adds a new check for deleted files in the printer linter. This will alert the user when a file has been deleted that could potentially disrupt upgrade scripts. An argument "--deleted" is also added to terminal.py to facilitate this new check. Additionally, the printer-linter version has been incremented to 0.1.2. CURA-10903 --- .github/workflows/printer-linter-pr-diagnose.yml | 6 +++++- .printer-linter | 1 + printer-linter/pyproject.toml | 2 +- printer-linter/src/printerlinter/factory.py | 2 +- .../src/printerlinter/linters/directory.py | 16 +++++++++++++++- printer-linter/src/terminal.py | 14 +++++++++++++- 6 files changed, 36 insertions(+), 5 deletions(-) diff --git a/.github/workflows/printer-linter-pr-diagnose.yml b/.github/workflows/printer-linter-pr-diagnose.yml index 57c3732d81..c9c5fb2f60 100644 --- a/.github/workflows/printer-linter-pr-diagnose.yml +++ b/.github/workflows/printer-linter-pr-diagnose.yml @@ -37,6 +37,10 @@ jobs: - name: Create results directory run: mkdir printer-linter-result + - name: Check Deleted Files(s) + if: env.GIT_DIFF + run: python printer-linter/src/terminal.py --deleted --report printer-linter-result/fixes.yml $(echo "$GIT_DIFF" | grep '^D') + - name: Diagnose file(s) if: env.GIT_DIFF && !env.MATCHED_FILES run: python printer-linter/src/terminal.py --diagnose --report printer-linter-result/fixes.yml ${{ env.GIT_DIFF_FILTERED }} @@ -56,5 +60,5 @@ jobs: uses: platisd/clang-tidy-pr-comments@bc0bb7da034a8317d54e7fe1e819159002f4cc40 with: github_token: ${{ secrets.GITHUB_TOKEN }} - clang_tidy_fixes: result.yml + clang_tidy_fixes: printer-linter-pr-post.yml request_changes: true diff --git a/.printer-linter b/.printer-linter index 3a42a5c033..e73e4f8ab2 100644 --- a/.printer-linter +++ b/.printer-linter @@ -3,6 +3,7 @@ checks: diagnostic-mesh-file-size: true diagnostic-definition-redundant-override: true diagnostic-resources-macos-app-directory-name: true + diagnostic-resource-file-deleted: true fixes: diagnostic-definition-redundant-override: true format: diff --git a/printer-linter/pyproject.toml b/printer-linter/pyproject.toml index c346dc0496..cde196225c 100644 --- a/printer-linter/pyproject.toml +++ b/printer-linter/pyproject.toml @@ -1,7 +1,7 @@ [project] name = "printerlinter" description = "Cura UltiMaker printer linting tool" -version = "0.1.1" +version = "0.1.2" authors = [ { name = "UltiMaker", email = "cura@ultimaker.com" } ] diff --git a/printer-linter/src/printerlinter/factory.py b/printer-linter/src/printerlinter/factory.py index 4473fb9a4e..0d706788fc 100644 --- a/printer-linter/src/printerlinter/factory.py +++ b/printer-linter/src/printerlinter/factory.py @@ -11,7 +11,7 @@ from .linters.directory import Directory def getLinter(file: Path, settings: dict) -> Optional[List[Linter]]: """ Returns a Linter depending on the file format """ if not file.exists(): - return None + return [Directory(file, settings)] if ".inst" in file.suffixes and ".cfg" in file.suffixes: return [Directory(file, settings), Profile(file, settings)] diff --git a/printer-linter/src/printerlinter/linters/directory.py b/printer-linter/src/printerlinter/linters/directory.py index 4ca299dad7..616c20af02 100644 --- a/printer-linter/src/printerlinter/linters/directory.py +++ b/printer-linter/src/printerlinter/linters/directory.py @@ -11,9 +11,12 @@ class Directory(Linter): super().__init__(file, settings) def check(self) -> Iterator[Diagnostic]: - if self._settings["checks"].get("diagnostic-resources-macos-app-directory-name", False): + if self._file.exists() and self._settings["checks"].get("diagnostic-resources-macos-app-directory-name", False): for check in self.checkForDotInDirName(): yield check + if self._settings["checks"].get("diagnostic-resource-file-deleted", False): + for check in self.checkFilesDeleted(): + yield check yield @@ -29,3 +32,14 @@ class Directory(Linter): ) yield + def checkFilesDeleted(self) -> Iterator[Diagnostic]: + """ Check if there is a file that is deleted, this causes upgrade scripts to not work properly """ + + yield Diagnostic( + file = self._file, + diagnostic_name = "diagnostic-resource-file-deleted", + message = f"File: {self._file} must not be deleted as it is not allowed. It will create issues upgrading Cura", + level = "Error", + offset = 1 + ) + yield \ No newline at end of file diff --git a/printer-linter/src/terminal.py b/printer-linter/src/terminal.py index fb5ee36bd0..fa2212285c 100644 --- a/printer-linter/src/terminal.py +++ b/printer-linter/src/terminal.py @@ -19,6 +19,7 @@ def main() -> None: parser.add_argument("--report", required=False, type=Path, help="Path where the diagnostic report should be stored") parser.add_argument("--format", action="store_true", help="Format the files") parser.add_argument("--diagnose", action="store_true", help="Diagnose the files") + parser.add_argument("--deleted", action="store_true", help="Check for deleted files") parser.add_argument("--fix", action="store_true", help="Attempt to apply the suggested fixes on the files") parser.add_argument("Files", metavar="F", type=Path, nargs="+", help="Files or directories to format") @@ -47,6 +48,18 @@ def main() -> None: print(f"Can't find the file: {file}") return + if args.deleted and files ==[]: + for file in args.Files: + deletedFiles = diagnoseIssuesWithFile(file, settings ) + full_body_check["Diagnostics"].extend([d.toDict() for d in deletedFiles]) + + results = yaml.dump(full_body_check, default_flow_style=False, indent=4, width=240) + + if report: + report.write_text(results) + else: + print(results) + if to_fix or to_diagnose: for file in files: diagnostics = diagnoseIssuesWithFile(file, settings) @@ -82,7 +95,6 @@ def diagnoseIssuesWithFile(file: Path, settings: dict) -> List[Diagnostic]: return linter_results - def applyFixesToFile(file, settings, full_body_check) -> None: if not file.exists(): return From 2f8829c2d120e7183742c2faf1d455fca07b9d1f Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Mon, 8 Apr 2024 10:42:17 +0200 Subject: [PATCH 06/17] GIT_DIFF not necessarity have the info of deleted files CURA-10903 --- .github/workflows/printer-linter-pr-diagnose.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/printer-linter-pr-diagnose.yml b/.github/workflows/printer-linter-pr-diagnose.yml index c9c5fb2f60..5aadffa5a9 100644 --- a/.github/workflows/printer-linter-pr-diagnose.yml +++ b/.github/workflows/printer-linter-pr-diagnose.yml @@ -38,8 +38,10 @@ jobs: run: mkdir printer-linter-result - name: Check Deleted Files(s) - if: env.GIT_DIFF - run: python printer-linter/src/terminal.py --deleted --report printer-linter-result/fixes.yml $(echo "$GIT_DIFF" | grep '^D') + + run: | + deletedFiles="$(git log -m -1 --name-status --pretty="format:" | awk '/^D/ {print $2}' | tr '\n' ' ')" + python printer-linter/src/terminal.py --deleted --report printer-linter-result/fixes.yml $(echo "$deletedFiles") - name: Diagnose file(s) if: env.GIT_DIFF && !env.MATCHED_FILES From 8b135b5562d5c6e54c83f959491e6ff392677ff4 Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Mon, 8 Apr 2024 10:50:24 +0200 Subject: [PATCH 07/17] Update command to retrieve deleted files CURA-10903 --- .github/workflows/printer-linter-pr-diagnose.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/printer-linter-pr-diagnose.yml b/.github/workflows/printer-linter-pr-diagnose.yml index 5aadffa5a9..70cdb668a9 100644 --- a/.github/workflows/printer-linter-pr-diagnose.yml +++ b/.github/workflows/printer-linter-pr-diagnose.yml @@ -38,9 +38,9 @@ jobs: run: mkdir printer-linter-result - name: Check Deleted Files(s) - run: | - deletedFiles="$(git log -m -1 --name-status --pretty="format:" | awk '/^D/ {print $2}' | tr '\n' ' ')" + - name: Get deleted files + git diff --name-only --diff-filter=D origin/main python printer-linter/src/terminal.py --deleted --report printer-linter-result/fixes.yml $(echo "$deletedFiles") - name: Diagnose file(s) From 13ad75a240fb9c02c80824e9de037551746bf02a Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Mon, 8 Apr 2024 10:52:17 +0200 Subject: [PATCH 08/17] fixing command CURA-10903 --- .github/workflows/printer-linter-pr-diagnose.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/printer-linter-pr-diagnose.yml b/.github/workflows/printer-linter-pr-diagnose.yml index 70cdb668a9..055303af45 100644 --- a/.github/workflows/printer-linter-pr-diagnose.yml +++ b/.github/workflows/printer-linter-pr-diagnose.yml @@ -39,7 +39,6 @@ jobs: - name: Check Deleted Files(s) run: | - - name: Get deleted files git diff --name-only --diff-filter=D origin/main python printer-linter/src/terminal.py --deleted --report printer-linter-result/fixes.yml $(echo "$deletedFiles") From c53e611c81c06f76fd2b7ea853102f56e01558c0 Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Mon, 8 Apr 2024 12:23:26 +0200 Subject: [PATCH 09/17] Update printer-linter workflow and fix file checks order CURA-10903 --- .github/workflows/printer-linter-pr-diagnose.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/printer-linter-pr-diagnose.yml b/.github/workflows/printer-linter-pr-diagnose.yml index 055303af45..86524ea956 100644 --- a/.github/workflows/printer-linter-pr-diagnose.yml +++ b/.github/workflows/printer-linter-pr-diagnose.yml @@ -18,6 +18,7 @@ jobs: - uses: technote-space/get-diff-action@v6 with: + DIFF_FILTER: AMRCD PATTERNS: | resources/+(extruders|definitions)/*.def.json resources/+(intent|quality|variants)/**/*.inst.cfg @@ -37,15 +38,14 @@ jobs: - name: Create results directory run: mkdir printer-linter-result - - name: Check Deleted Files(s) - run: | - git diff --name-only --diff-filter=D origin/main - python printer-linter/src/terminal.py --deleted --report printer-linter-result/fixes.yml $(echo "$deletedFiles") - - name: Diagnose file(s) if: env.GIT_DIFF && !env.MATCHED_FILES run: python printer-linter/src/terminal.py --diagnose --report printer-linter-result/fixes.yml ${{ env.GIT_DIFF_FILTERED }} + - name: Check Deleted Files(s) + if: env.GIT_DIFF + run: python printer-linter/src/terminal.py --deleted --report printer-linter-result/fixes.yml ${{ env.GIT_DIFF_FILTERED }} + - name: Save PR metadata run: | echo ${{ github.event.number }} > printer-linter-result/pr-id.txt @@ -61,5 +61,5 @@ jobs: uses: platisd/clang-tidy-pr-comments@bc0bb7da034a8317d54e7fe1e819159002f4cc40 with: github_token: ${{ secrets.GITHUB_TOKEN }} - clang_tidy_fixes: printer-linter-pr-post.yml + clang_tidy_fixes: results.yml request_changes: true From b3a25893fc4d2ab776f93352b320c3a32f586a57 Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Mon, 8 Apr 2024 12:25:10 +0200 Subject: [PATCH 10/17] fix results to result CURA-10903 --- .github/workflows/printer-linter-pr-diagnose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/printer-linter-pr-diagnose.yml b/.github/workflows/printer-linter-pr-diagnose.yml index 86524ea956..2846cdb603 100644 --- a/.github/workflows/printer-linter-pr-diagnose.yml +++ b/.github/workflows/printer-linter-pr-diagnose.yml @@ -61,5 +61,5 @@ jobs: uses: platisd/clang-tidy-pr-comments@bc0bb7da034a8317d54e7fe1e819159002f4cc40 with: github_token: ${{ secrets.GITHUB_TOKEN }} - clang_tidy_fixes: results.yml + clang_tidy_fixes: result.yml request_changes: true From 29617ea22f2ee21f5bf1d1bf88e64745f672d620 Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Mon, 8 Apr 2024 12:42:33 +0200 Subject: [PATCH 11/17] removed path with parent because it doesn't exist anymore --- printer-linter/src/printerlinter/linters/directory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/printer-linter/src/printerlinter/linters/directory.py b/printer-linter/src/printerlinter/linters/directory.py index 616c20af02..e1b04c84f6 100644 --- a/printer-linter/src/printerlinter/linters/directory.py +++ b/printer-linter/src/printerlinter/linters/directory.py @@ -36,7 +36,7 @@ class Directory(Linter): """ Check if there is a file that is deleted, this causes upgrade scripts to not work properly """ yield Diagnostic( - file = self._file, + file = self._file.parent, diagnostic_name = "diagnostic-resource-file-deleted", message = f"File: {self._file} must not be deleted as it is not allowed. It will create issues upgrading Cura", level = "Error", From 561a40d000a94636221c032587e9f6daa55109f0 Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Tue, 9 Apr 2024 11:33:24 +0200 Subject: [PATCH 12/17] Implement GitComment class and update workflow files A new GitComment class was implemented to replace Diagnostic for deleted file checks. As part of this change, both main workflow files (printer-linter-pr-diagnose.yml and printer-linter-pr-post.yml) have been updated to accommodate this new class. Also, reports now use 'comment.md' instead of 'fixes.yml'. All of this is ultimately geared at improving diagnostic functionality and allowing deleted file checks to output directly to a Git comment. CURA-10903 --- .../workflows/printer-linter-pr-diagnose.yml | 2 +- .github/workflows/printer-linter-pr-post.yml | 7 +++++++ printer-linter/src/printerlinter/diagnostic.py | 10 ++++++++++ .../src/printerlinter/linters/directory.py | 18 ++++++------------ printer-linter/src/terminal.py | 7 ++++--- 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/.github/workflows/printer-linter-pr-diagnose.yml b/.github/workflows/printer-linter-pr-diagnose.yml index 2846cdb603..1be20c4548 100644 --- a/.github/workflows/printer-linter-pr-diagnose.yml +++ b/.github/workflows/printer-linter-pr-diagnose.yml @@ -44,7 +44,7 @@ jobs: - name: Check Deleted Files(s) if: env.GIT_DIFF - run: python printer-linter/src/terminal.py --deleted --report printer-linter-result/fixes.yml ${{ env.GIT_DIFF_FILTERED }} + run: python printer-linter/src/terminal.py --deleted --report printer-linter-result/comment.md ${{ env.GIT_DIFF_FILTERED }} - name: Save PR metadata run: | diff --git a/.github/workflows/printer-linter-pr-post.yml b/.github/workflows/printer-linter-pr-post.yml index 81dbf96469..94fe810c43 100644 --- a/.github/workflows/printer-linter-pr-post.yml +++ b/.github/workflows/printer-linter-pr-post.yml @@ -72,6 +72,13 @@ jobs: mkdir printer-linter-result unzip printer-linter-result.zip -d printer-linter-result + - name: Run PR Comments + uses: peter-evans/find-comment@v3 + with: + issue-number: ${{ github.event.pull_request.number }} + comment-author: 'github-actions[bot]' + body-path: printer-linter-result/comment.md + - name: Run clang-tidy-pr-comments action uses: platisd/clang-tidy-pr-comments@bc0bb7da034a8317d54e7fe1e819159002f4cc40 with: diff --git a/printer-linter/src/printerlinter/diagnostic.py b/printer-linter/src/printerlinter/diagnostic.py index 27f4fdd14a..1ef8bef638 100644 --- a/printer-linter/src/printerlinter/diagnostic.py +++ b/printer-linter/src/printerlinter/diagnostic.py @@ -32,3 +32,13 @@ class Diagnostic: }, "Level": self.level } + +class GitComment: + def __init__(self, comment: str) -> None: + """ + @param comment: The comment text. + """ + self.comment = comment + + def toDict(self) -> Dict[str, Any]: + return self.comment \ No newline at end of file diff --git a/printer-linter/src/printerlinter/linters/directory.py b/printer-linter/src/printerlinter/linters/directory.py index e1b04c84f6..d8626b1be4 100644 --- a/printer-linter/src/printerlinter/linters/directory.py +++ b/printer-linter/src/printerlinter/linters/directory.py @@ -1,7 +1,7 @@ from pathlib import Path from typing import Iterator -from ..diagnostic import Diagnostic +from ..diagnostic import Diagnostic, GitComment from .linter import Linter @@ -14,7 +14,7 @@ class Directory(Linter): if self._file.exists() and self._settings["checks"].get("diagnostic-resources-macos-app-directory-name", False): for check in self.checkForDotInDirName(): yield check - if self._settings["checks"].get("diagnostic-resource-file-deleted", False): + elif self._settings["checks"].get("diagnostic-resource-file-deleted", False): for check in self.checkFilesDeleted(): yield check @@ -32,14 +32,8 @@ class Directory(Linter): ) yield - def checkFilesDeleted(self) -> Iterator[Diagnostic]: - """ Check if there is a file that is deleted, this causes upgrade scripts to not work properly """ - - yield Diagnostic( - file = self._file.parent, - diagnostic_name = "diagnostic-resource-file-deleted", - message = f"File: {self._file} must not be deleted as it is not allowed. It will create issues upgrading Cura", - level = "Error", - offset = 1 - ) + def checkFilesDeleted(self) -> Iterator[GitComment]: + if not self._file.exists(): + """ Check if there is a file that is deleted, this causes upgrade scripts to not work properly """ + yield GitComment( f"File: {self._file} must not be deleted as it is not allowed. It will create issues upgrading Cura" ) yield \ No newline at end of file diff --git a/printer-linter/src/terminal.py b/printer-linter/src/terminal.py index fa2212285c..b7c1607660 100644 --- a/printer-linter/src/terminal.py +++ b/printer-linter/src/terminal.py @@ -42,18 +42,19 @@ def main() -> None: settings = yaml.load(f, yaml.FullLoader) full_body_check = {"Diagnostics": []} + comments_check = {"Git Comment": []} for file in files: if not path.exists(file): print(f"Can't find the file: {file}") return - if args.deleted and files ==[]: + if args.deleted: for file in args.Files: deletedFiles = diagnoseIssuesWithFile(file, settings ) - full_body_check["Diagnostics"].extend([d.toDict() for d in deletedFiles]) + comments_check["GitComment"].extend([d.toDict() for d in deletedFiles]) - results = yaml.dump(full_body_check, default_flow_style=False, indent=4, width=240) + results = yaml.dump(comments_check, default_flow_style=False, indent=4, width=240) if report: report.write_text(results) From bb94ce9e753ff18fcbd155843fbca3571e280c1d Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Tue, 9 Apr 2024 11:37:42 +0200 Subject: [PATCH 13/17] fixing typo checks CURA-10903 --- printer-linter/src/printerlinter/linters/defintion.py | 2 +- printer-linter/src/terminal.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/printer-linter/src/printerlinter/linters/defintion.py b/printer-linter/src/printerlinter/linters/defintion.py index bf9a39452a..2d68e20db9 100644 --- a/printer-linter/src/printerlinter/linters/defintion.py +++ b/printer-linter/src/printerlinter/linters/defintion.py @@ -90,7 +90,7 @@ class Definition(Linter): yield Diagnostic( file=self._file, - diagnostic_name="diagnostic-definition-redundant-override", + diagnostic_name="diagnostic-material-temperature-defined", message=f"Overriding {key} as it belongs to material temperature catagory and shouldn't be placed in machine definitions", level="Warning", offset=found.span(0)[0], diff --git a/printer-linter/src/terminal.py b/printer-linter/src/terminal.py index b7c1607660..3de4c3a979 100644 --- a/printer-linter/src/terminal.py +++ b/printer-linter/src/terminal.py @@ -52,7 +52,7 @@ def main() -> None: if args.deleted: for file in args.Files: deletedFiles = diagnoseIssuesWithFile(file, settings ) - comments_check["GitComment"].extend([d.toDict() for d in deletedFiles]) + comments_check["Git Comment"].extend([d.toDict() for d in deletedFiles]) results = yaml.dump(comments_check, default_flow_style=False, indent=4, width=240) From d52cd7885239c903a57e53f1713bfb93b0d87b12 Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Wed, 10 Apr 2024 14:01:17 +0200 Subject: [PATCH 14/17] Update GitHub workflow to use create-or-update-comment action In this update, the GitHub workflow now uses the 'create-or-update-comment' action instead of the 'find-comment' action for PR comments. Additionally, a step has been added to get the PR number. This change is expected to facilitate more efficient PR commenting and handling. CURa-10903 --- .github/workflows/printer-linter-pr-post.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/printer-linter-pr-post.yml b/.github/workflows/printer-linter-pr-post.yml index 94fe810c43..fd6e63ce78 100644 --- a/.github/workflows/printer-linter-pr-post.yml +++ b/.github/workflows/printer-linter-pr-post.yml @@ -72,12 +72,15 @@ jobs: mkdir printer-linter-result unzip printer-linter-result.zip -d printer-linter-result + - name: Get PR Number + id: get-pr-number + uses: mgaitan/gha-get-pr-number + - name: Run PR Comments - uses: peter-evans/find-comment@v3 + uses: peter-evans/create-or-update-comment@v4 with: - issue-number: ${{ github.event.pull_request.number }} - comment-author: 'github-actions[bot]' - body-path: printer-linter-result/comment.md + issue-number: ${{ steps.get-pr-number.outputs.number }} + body-path: 'printer-linter-result/comment.md' - name: Run clang-tidy-pr-comments action uses: platisd/clang-tidy-pr-comments@bc0bb7da034a8317d54e7fe1e819159002f4cc40 From a965392559ba4106f75c19ef22c54ea48297bd6d Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Wed, 10 Apr 2024 14:16:25 +0200 Subject: [PATCH 15/17] Update method for getting PR number and modify output report The code updates the way we fetch the Pull Request number in the Github action. Rather than using an external action, we directly make use of Github's CLI, which helps improve the speed and reliability of the workflow. Additionally, the output report's format in terminal.py, previously named as "Git Comment", has been changed to "Error Files" to better reflect the information it carries. CURA-10903 --- .github/workflows/printer-linter-pr-post.yml | 10 ++++++---- printer-linter/src/terminal.py | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/workflows/printer-linter-pr-post.yml b/.github/workflows/printer-linter-pr-post.yml index fd6e63ce78..a71449299d 100644 --- a/.github/workflows/printer-linter-pr-post.yml +++ b/.github/workflows/printer-linter-pr-post.yml @@ -72,14 +72,16 @@ jobs: mkdir printer-linter-result unzip printer-linter-result.zip -d printer-linter-result - - name: Get PR Number - id: get-pr-number - uses: mgaitan/gha-get-pr-number + - name: Get Pull Request Number + id: pr + run: echo "::set-output name=pull_request_number::$(gh pr view --json number -q .number || echo "")" + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Run PR Comments uses: peter-evans/create-or-update-comment@v4 with: - issue-number: ${{ steps.get-pr-number.outputs.number }} + issue-number: ${{ steps.pr.outputs.pull_request_number }} body-path: 'printer-linter-result/comment.md' - name: Run clang-tidy-pr-comments action diff --git a/printer-linter/src/terminal.py b/printer-linter/src/terminal.py index 3de4c3a979..d8ef7a77cb 100644 --- a/printer-linter/src/terminal.py +++ b/printer-linter/src/terminal.py @@ -42,7 +42,7 @@ def main() -> None: settings = yaml.load(f, yaml.FullLoader) full_body_check = {"Diagnostics": []} - comments_check = {"Git Comment": []} + comments_check = {"Error Files": []} for file in files: if not path.exists(file): @@ -52,7 +52,7 @@ def main() -> None: if args.deleted: for file in args.Files: deletedFiles = diagnoseIssuesWithFile(file, settings ) - comments_check["Git Comment"].extend([d.toDict() for d in deletedFiles]) + comments_check["Error Files"].extend([d.toDict() for d in deletedFiles]) results = yaml.dump(comments_check, default_flow_style=False, indent=4, width=240) From c6ff0d3030443647f3106796230f52b4739f4dfa Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Wed, 10 Apr 2024 14:50:26 +0200 Subject: [PATCH 16/17] Enhance error reporting for deleted files The code now checks if a requested file is present in the files list before diagnosing issues and generating error reports for it. It will help prevent attempting to diagnose or report on files that do not exist or deleted, thus enhancing efficiency and preventing possible error generation CURA-10903 --- .../src/printerlinter/linters/directory.py | 2 +- printer-linter/src/terminal.py | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/printer-linter/src/printerlinter/linters/directory.py b/printer-linter/src/printerlinter/linters/directory.py index d8626b1be4..e77cac96b9 100644 --- a/printer-linter/src/printerlinter/linters/directory.py +++ b/printer-linter/src/printerlinter/linters/directory.py @@ -35,5 +35,5 @@ class Directory(Linter): def checkFilesDeleted(self) -> Iterator[GitComment]: if not self._file.exists(): """ Check if there is a file that is deleted, this causes upgrade scripts to not work properly """ - yield GitComment( f"File: {self._file} must not be deleted as it is not allowed. It will create issues upgrading Cura" ) + yield GitComment( f'File: **{self._file}** must not be deleted as it is not allowed. It will create issues upgrading Cura' ) yield \ No newline at end of file diff --git a/printer-linter/src/terminal.py b/printer-linter/src/terminal.py index d8ef7a77cb..d93372571f 100644 --- a/printer-linter/src/terminal.py +++ b/printer-linter/src/terminal.py @@ -51,15 +51,16 @@ def main() -> None: if args.deleted: for file in args.Files: - deletedFiles = diagnoseIssuesWithFile(file, settings ) - comments_check["Error Files"].extend([d.toDict() for d in deletedFiles]) + if file not in files: + deletedFiles = diagnoseIssuesWithFile(file, settings) + comments_check["Error Files"].extend([d.toDict() for d in deletedFiles]) - results = yaml.dump(comments_check, default_flow_style=False, indent=4, width=240) + results = yaml.dump(comments_check, default_flow_style=False, indent=4, width=240) - if report: - report.write_text(results) - else: - print(results) + if report: + report.write_text(results) + else: + print(results) if to_fix or to_diagnose: for file in files: From 3016820bee34375547a0ccfd812b75a95ee7386e Mon Sep 17 00:00:00 2001 From: Saumya Jain Date: Thu, 11 Apr 2024 09:47:40 +0200 Subject: [PATCH 17/17] fixing printer linter PR post --- .github/workflows/printer-linter-pr-post.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/printer-linter-pr-post.yml b/.github/workflows/printer-linter-pr-post.yml index a71449299d..8bb1aaf028 100644 --- a/.github/workflows/printer-linter-pr-post.yml +++ b/.github/workflows/printer-linter-pr-post.yml @@ -39,6 +39,11 @@ jobs: echo "pr_id=$(cat printer-linter-result/pr-id.txt)" >> $GITHUB_ENV echo "pr_head_repo=$(cat printer-linter-result/pr-head-repo.txt)" >> $GITHUB_ENV echo "pr_head_ref=$(cat printer-linter-result/pr-head-ref.txt)" >> $GITHUB_ENV + if [[ -f "printer-linter-result/comment.md" ]]; then + echo "commentFileExists=true" >> $GITHUB_ENV + else + echo "commentFileExists=false" >> $GITHUB_ENV + fi - uses: actions/checkout@v3 with: @@ -72,16 +77,11 @@ jobs: mkdir printer-linter-result unzip printer-linter-result.zip -d printer-linter-result - - name: Get Pull Request Number - id: pr - run: echo "::set-output name=pull_request_number::$(gh pr view --json number -q .number || echo "")" - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - name: Run PR Comments + if: env.commentFileExists == 'true' uses: peter-evans/create-or-update-comment@v4 with: - issue-number: ${{ steps.pr.outputs.pull_request_number }} + issue-number: ${{ env.pr_id }} body-path: 'printer-linter-result/comment.md' - name: Run clang-tidy-pr-comments action