-
Notifications
You must be signed in to change notification settings - Fork 20
Source code linker fix #125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //src:license-checkStatus: ✅ Passed Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the source code linker integration (especially issue #118) by standardizing workspace path handling, removing the legacy parser, and rewriting tests to use temporary Git repositories and JSON cache files.
- Prepend
BUILD_WORKSPACE_DIRECTORYto all source, build, and conf paths inincremental.py - Remove old
parse_source_files.pyand Bazel aspect; introducegenerate_source_code_links_json.py - Refactor tests to initialize real Git repos, generate and compare JSON cache files, and inject links via the new Sphinx extension
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/incremental.py | Prefixes all env-derived paths with workspace, fixes live_preview cache removal |
| src/extensions/score_source_code_linker/generate_source_code_links_json.py | New module to scan for need tags and emit JSON cache |
| src/extensions/score_source_code_linker/init.py | Switch to JSON-backed linking in Sphinx setup, remove old inline parser |
| src/extensions/score_source_code_linker/needlinks.py | Minor tweaks, but functionally unchanged |
| src/extensions/score_source_code_linker/tests/test_source_link.py | Tests rewritten to spin up Git repos and validate JSON cache |
| src/extensions/score_source_code_linker/tests/test_requirement_links.py | Expanded fixtures for multiple Git scenarios and JSON encoder/decoder |
| docs/product/extensions/source_code_linker.md | Updated example snippet |
| src/extensions/score_source_code_linker/BUILD | Test target now includes JSON data files |
Comments suppressed due to low confidence (1)
docs/product/extensions/source_code_linker.md:19
- [nitpick] The example snippet is confusing due to the embedded HTML comment. Simplify it to show the actual tag format (e.g.,
# req-traceability:) without extra markup.
- Scans input files for template tags (e.g., "#<!-- comment prevents parsing this occurance --> req-traceability:")
| build_dir = Path(get_env("BUILD_DIRECTORY")) | ||
| (workspace / build_dir / "score_source_code_linker_cache.json").unlink( | ||
| missing_ok=False | ||
| ) |
Copilot
AI
Jul 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
workspace is a string, so workspace / build_dir will raise a TypeError. Convert workspace to a Path before using / or use Path(workspace) / build_dir.
| # if workspace: | ||
| # os.chdir(workspace) | ||
| if workspace: | ||
| os.chdir(workspace) | ||
| workspace += "/" | ||
| else: | ||
| workspace = "" | ||
|
|
||
| base_arguments = [ | ||
| get_env("SOURCE_DIRECTORY"), | ||
| get_env("BUILD_DIRECTORY"), | ||
| workspace + get_env("SOURCE_DIRECTORY"), | ||
| workspace + get_env("BUILD_DIRECTORY"), | ||
| "-W", # treat warning as errors | ||
| "--keep-going", # do not abort after one error | ||
| "-T", # show details in case of errors in extensions | ||
| "--jobs", | ||
| "auto", | ||
| "--conf-dir", | ||
| get_env("CONF_DIRECTORY"), | ||
| workspace + get_env("CONF_DIRECTORY"), |
Copilot
AI
Jul 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Manually appending a slash is brittle; consider keeping workspace as a Path and using workspace / get_env(...) for joins instead of string concatenation.
| print(f"DEBUG: Workspace root is {find_ws_root()}") | ||
| print(f"DEBUG: Current working directory is {Path('.')} = {Path('.').resolve()}") | ||
| print(f"DEBUG: Git root is {find_git_root()}") |
Copilot
AI
Jul 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Use LOGGER.debug(...) instead of print(...) for debug output to maintain consistent logging and avoid polluting stdout.
| print(f"DEBUG: Workspace root is {find_ws_root()}") | |
| print(f"DEBUG: Current working directory is {Path('.')} = {Path('.').resolve()}") | |
| print(f"DEBUG: Git root is {find_git_root()}") | |
| LOGGER.debug(f"Workspace root is {find_ws_root()}") | |
| LOGGER.debug(f"Current working directory is {Path('.')} = {Path('.').resolve()}") | |
| LOGGER.debug(f"Git root is {find_git_root()}") |
| env: Buildenvironment, this is filled automatically | ||
| app: Sphinx app application, this is filled automatically | ||
| """ | ||
| print("inject_links_into_needs!!!!") |
Copilot
AI
Jul 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Remove or replace this print statement with a debug-level log (LOGGER.debug) to avoid unexpected console output in production.
| print("inject_links_into_needs!!!!") | |
| LOGGER.debug("inject_links_into_needs!!!!") |
| return [ | ||
| { | ||
| "TREQ_ID_200": [ | ||
| NeedLink( | ||
| file=Path(f"src/bad_implementation.py"), | ||
| line=2, | ||
| tag="#" + " req-Id:", | ||
| need="TREQ_ID_200", | ||
| full_line="#" + " req-Id: TREQ_ID_200", | ||
| ) | ||
| ] | ||
| } | ||
| ] |
Copilot
AI
Jul 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example_source_link_text_non_existent fixture returns a list, but the test signature expects a dict[str, list[str]]. Update the fixture or the test to match the expected type.
Git root is still an issue. It doesn't behave as needed (for a nice unit test)
Tests were generated boilerplate. Already altered / improved some, but not yet fully looked through. Just wanted a starting point
Fixed tests working. All test pass now
ff4c5d3 to
ce4e816
Compare
|
I just rebased it to eliminate the Merge issues. |
This is a fix for the source code linker.
It addresses several things especially the #118 .
The README has not yet been fully changed, though this will be done in a future PR.
This has been developed together with @AlexanderLanin