Fix linting errors part 2 by Aymen-Soussi-01 · Pull Request #211 · eclipse-score/docs-as-code

Aymen-Soussi-01 · 2025-08-14T16:00:10Z

This PR resolves a total of 32 remaining linting errors, addressing various code quality and style improvements. The following changes were made:

Fixed line length violations to comply with style guidelines.
Removed unnecessary f prefix from strings where no interpolation occurs (e.g., f"hello" → "hello").
Eliminated redundant else blocks following a return statement.
Removed unnecessary default mode argument "r" when opening files (e.g., open("module.bazel", "r") → open("module.bazel")).
Replaced assert False statements with more descriptive and proper error handling.
Removed redundant else after continue statements.
Used contextlib.suppress(FileNotFoundError) instead of try/except/pass for cleaner exception handling.
Fixed blind assert Exception usage by specifying explicit exception types.
Moved function calls out of default parameter values and into the function body.
Refactored several functions to reduce cyclomatic complexity while maintaining original functionality and comments.

Note 1: All builds, documentation generation, and tests run successfully after these changes. Note 2: We got a new list of 66 different errors and warnings after solving the initial first list .

close: #205

github-actions · 2025-08-14T16:02:54Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //src:license-check

Status: ✅ Passed

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: 96d92e66-fc9b-4db2-b681-8b5fc93e42a7
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: src
Analyzing: target //src:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //src:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //src:license-check (73 packages loaded, 9 targets configured)

Analyzing: target //src:license-check (124 packages loaded, 2291 targets configured)

Analyzing: target //src:license-check (129 packages loaded, 2423 targets configured)

Analyzing: target //src:license-check (137 packages loaded, 3442 targets configured)

INFO: Analyzed target //src:license-check (137 packages loaded, 4488 targets configured).
INFO: Found 1 target...
Target //src:license.check.license_check up-to-date:
  bazel-bin/src/license.check.license_check
  bazel-bin/src/license.check.license_check.jar
INFO: Elapsed time: 15.444s, Critical Path: 0.45s
INFO: 13 processes: 4 disk cache hit, 9 internal.
INFO: Build completed successfully, 13 total actions
INFO: Running command line: bazel-bin/src/license.check.license_check src/formatted.txt <args omitted>
[main] INFO Querying Eclipse Foundation for license data for 84 items.
[main] INFO Found 58 items.
[main] INFO Querying ClearlyDefined for license data for 26 items.
[main] INFO Found 26 items.
[main] INFO Vetted license information was found for all content. No further investigation is required.

github-actions · 2025-08-14T16:03:51Z

The created documentation from the pull request is available at: docu-html

AlexanderLanin · 2025-08-14T19:29:29Z

fun fact: github understands the error outputs!!

AlexanderLanin · 2025-08-14T19:25:19Z

+        return [raw_value]
+    if isinstance(raw_value, list) and all(isinstance(v, str) for v in raw_value):
+        return raw_value
+    return [str(raw_value)]


Not sure about that last part here... how about

Suggested change

return [str(raw_value)]

raise ValueError("unexpected types")

Fixed. Please resolve.

AlexanderLanin · 2025-08-14T19:32:08Z

@MaximilianSoerenPollak as you also have another huge PR: which one should we merge first?

Copilot

Pull Request Overview

This PR addresses linting errors and improves code quality by fixing various style and maintainability issues. The changes focus on eliminating redundant code constructs, improving error handling, and reducing cyclomatic complexity while maintaining functionality.

Removed redundant f-string prefixes, unnecessary else blocks after return/continue statements
Replaced generic exception handling with specific exception types and improved error messages
Refactored complex functions to reduce cyclomatic complexity through helper function extraction

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/tests/test_consumer.py	Removes redundant else blocks, f-string prefixes, file mode arguments, and improves error handling
src/find_runfiles/test_find_runfiles.py	Fixes line length violations by breaking long assertion statements
src/extensions/score_source_code_linker/tests/test_source_link.py	Improves exception handling with contextlib.suppress and removes f-string prefix
src/extensions/score_source_code_linker/tests/test_requirement_links.py	Replaces generic Exception with specific subprocess.CalledProcessError and removes unused variable
src/extensions/score_source_code_linker/init.py	Moves default parameter values into function body and simplifies conditional logic
src/extensions/score_metamodel/checks/check_options.py	Reduces cyclomatic complexity by extracting helper functions
src/extensions/score_metamodel/checks/attributes_format.py	Simplifies complex conditional expressions and removes dictionary keys() call
src/extensions/score_metamodel/init.py	Removes unused import
src/extensions/score_draw_uml_funcs/init.py	Significantly reduces cyclomatic complexity by extracting multiple helper functions

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

MaximilianSoerenPollak · 2025-08-14T20:01:05Z

@MaximilianSoerenPollak as you also have another huge PR: which one should we merge first?

I will finish up the Lib PR #207 , I think that should be merged first as to not delay it any further with big merge issues.

MaximilianSoerenPollak · 2025-08-15T09:58:33Z


            need_as_dict["source_code_link"] = ", ".join(
-                f"{get_github_link(n)}<>{n.file}:{n.line}" for n in needlinks
+                f"{get_github_link(None, n)}<>{n.file}:{n.line}" for n in needlinks


No, this takes only one argument. No 2nd one.

Take a look at helper_lib/__init__.py where this function is defined.

This what was making the build failed we are using the function get_github_link defined in the same file in line 207 it takes 2 arguments as you see

As said.

The one in helper_lib is the right one/ fixed one. Use that, delete the other definition.
But take over your changes that fix linter warnings to the helper_lib functions

I used the same function as in the main now, and no more errors related to it are there in the build docs or in tests

AlexanderLanin · 2025-08-15T10:00:04Z

How come linting fixes introduce so much new code? 😆

Aymen-Soussi-01 · 2025-08-15T12:06:52Z

PR Updates: All problems are solved, and documentation and test builds are running with no issues. PR can be merged.

…ests

MaximilianSoerenPollak · 2025-08-15T13:26:21Z

+    ) -> tuple[
+        str, str, dict[str, str], dict[str, list[str]], dict[str, str], list[str]
+    ]:


Nit (fix in new PR's):

This return type is absolutley abysmal.
Truly helps noone. Should have a custom type here

MaximilianSoerenPollak · 2025-08-15T13:29:37Z

+        length = 0
+        if "example_feature" not in need["id"]:
+            length = len(need["id"])
+        else:
+            length = len(need["id"]) - 17


Nit fix in next pr:

Suggested change

length = 0

if "example_feature" not in need["id"]:

length = len(need["id"])

else:

length = len(need["id"]) - 17

length = len(need["id"])

if "example_feature" in need["id"]:

length -= 17

MaximilianSoerenPollak · 2025-08-15T13:32:54Z

+        try:
+            values = _normalize_values(raw_value)
+        except ValueError as err:
+            raise ValueError(
+                f"An Attribute inside need {need['id']} is "
+                "not of type str. Only Strings are allowed"
+            ) from err


Nit (fix in next PR):

Add comment explaining why we did this.

Suggested change

try:

values = _normalize_values(raw_value)

except ValueError as err:

raise ValueError(

f"An Attribute inside need {need['id']} is "

"not of type str. Only Strings are allowed"

) from err

# Try except used to add more context to Error without passing variables just for that to function

try:

values = _normalize_values(raw_value)

except ValueError as err:

raise ValueError(

f"An Attribute inside need {need['id']} is "

"not of type str. Only Strings are allowed"

) from err

MaximilianSoerenPollak · 2025-08-15T13:33:19Z

@@ -156,7 +156,7 @@ def test_invalid_option_type(self):
            target_id="wf_req__001",


Nit (fix in next PR):

Add a test that now tests the new 'value error' path we have introduced

MaximilianSoerenPollak · 2025-08-15T13:33:59Z

        # This fixes the "no such file or directory" error in Bazel
-        original_cwd = None
-        try:
+        with contextlib.suppress(FileNotFoundError):


Nit (fix in next PR):

Comment what this does briefly as people might not be familliar with this.

I kind of disagree. We should not explain standard python functionality. Python programmers must know python.
(But we should explain why we ignore the exception)

That does make more sense, true. Let's do that.

MaximilianSoerenPollak · 2025-08-15T13:35:14Z

    if git_root is None:
-        assert False, "Git root was none"
+        raise RuntimeError("Git root was not found")


Nit (fix enxt pr):

Suggested change

if git_root is None:

assert False, "Git root was none"

raise RuntimeError("Git root was not found")

assert git_root is None, "Git root was not found"

* Fix and simplify complexity for weak content check * handle edge cases to remove matching between "something" and "thing" * Correct PR #211 remaining comments

* Solve most of the linting errors * Reduce complexities of functions * Remove unused and repeated function from source code linker and fix tests

* Fix and simplify complexity for weak content check * handle edge cases to remove matching between "something" and "thing" * Correct PR eclipse-score#211 remaining comments

Aymen-Soussi-01 requested review from AlexanderLanin, MaximilianSoerenPollak and dcalavrezo-qorix as code owners August 14, 2025 16:00

github-project-automation Bot added this to INF - Infrastructure Community Aug 14, 2025

github-project-automation Bot moved this to Draft in INF - Infrastructure Community Aug 14, 2025

Aymen-Soussi-01 self-assigned this Aug 14, 2025

Aymen-Soussi-01 requested a review from nicu1989 August 14, 2025 16:01

AlexanderLanin previously approved these changes Aug 14, 2025

View reviewed changes

AlexanderLanin requested a review from Copilot August 14, 2025 19:34

Copilot AI reviewed Aug 14, 2025

View reviewed changes

Comment thread src/extensions/score_source_code_linker/tests/test_source_link.py Outdated

Comment thread src/extensions/score_metamodel/checks/check_options.py

Aymen-Soussi-01 dismissed AlexanderLanin’s stale review via 0b2de82 August 15, 2025 08:25

Aymen-Soussi-01 added 3 commits August 15, 2025 10:47

Solve most of the linting errors

5546f01

Reduce complexities of functions

9b31c42

Fix review comments

9d092cb

Aymen-Soussi-01 force-pushed the fix-linting-errors-part-2 branch from 0b2de82 to 9d092cb Compare August 15, 2025 08:51

Aymen-Soussi-01 added 4 commits August 15, 2025 11:25

Correct new linting errors after rebase the main

5e14aeb

Fix formatting errors

3a33cd2

return back get_github_base_url function and remove it's import

b858089

Fix build docs errors

d26551a

MaximilianSoerenPollak reviewed Aug 15, 2025

View reviewed changes

Fix get_github_link errors

104f405

Remove unused and repeated function from source code linker and fix t…

74a34d9

…ests

MaximilianSoerenPollak reviewed Aug 15, 2025

View reviewed changes

MaximilianSoerenPollak approved these changes Aug 15, 2025

View reviewed changes

MaximilianSoerenPollak merged commit 90a126e into eclipse-score:main Aug 15, 2025
7 of 8 checks passed

github-project-automation Bot moved this from Draft to Done in INF - Infrastructure Community Aug 15, 2025

Aymen-Soussi-01 added a commit to Aymen-Soussi-01/docs-as-code that referenced this pull request Aug 18, 2025

Correct PR eclipse-score#211 remaining comments

cdaf931

Aymen-Soussi-01 mentioned this pull request Aug 18, 2025

Fix new needs weak content not detected #214

Merged

Aymen-Soussi-01 added a commit to Aymen-Soussi-01/docs-as-code that referenced this pull request Aug 18, 2025

Correct PR eclipse-score#211 remaining comments

7ab506f

		@@ -156,7 +156,7 @@ def test_invalid_option_type(self):
		target_id="wf_req__001",

Conversation

Aymen-Soussi-01 commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions Bot commented Aug 14, 2025

Uh oh!

AlexanderLanin commented Aug 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderLanin commented Aug 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

MaximilianSoerenPollak commented Aug 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderLanin commented Aug 15, 2025

Uh oh!

Aymen-Soussi-01 commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaximilianSoerenPollak Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Aymen-Soussi-01 commented Aug 14, 2025 •

edited

Loading

github-actions Bot commented Aug 14, 2025 •

edited

Loading

Aymen-Soussi-01 commented Aug 15, 2025 •

edited

Loading

MaximilianSoerenPollak Aug 15, 2025 •

edited

Loading