Adding helper_lib#207
Conversation
|
/consumer-test |
License Check Results🚀 The license check job ran with the Bazel command: bazel run //src:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
|
Can you check against eclipse-score/tooling#30 please |
bfd3507 to
5afbace
Compare
| @@ -356,35 +359,35 @@ def test_group_by_need_empty_list(): | |||
| def test_parse_git_output_ssh_format(): | |||
There was a problem hiding this comment.
move these tests? (separate PR?)
There was a problem hiding this comment.
Yes, need to move these yep.
| # fallback to cwd when building with python -m sphinx docs _build -T | ||
| if recoursion_break: | ||
| return None |
There was a problem hiding this comment.
Is it possible to write that loop somehow differently? I still don't get it. And there is no fallback to cwd?
There was a problem hiding this comment.
Basically,
In Esbonio or other executions it can happen that it reaches the home directory '/' and there is still no .git.
If this happens it would just loop indifferently without stopping. Therefore if we found the '/' then we tell it that it already did find it and break.
There might be a better way to write this, I have another look at it.
There was a problem hiding this comment.
Consider while + else. No idea whether that will make it better, but it sounds like a perfect fit.
| Unlike other funcions, this function behaves the same for the following commands: | ||
|
|
||
| 'bazel run' => str | ||
| 'bazel build' => str |
There was a problem hiding this comment.
bazel build should not have .git in the sandbox? And it could run remotely where there is definitely no .git.
There was a problem hiding this comment.
I did print the statements inside of the conf.py and accumulated the behaviour via that.
I have not tested it yet as a remote / imported thing.
|
|
||
| def get_github_repo_info(git_root_cwd: Path) -> str: | ||
| """ | ||
| Unlike other funcions, this function behaves the same for the following commands: |
There was a problem hiding this comment.
| Unlike other funcions, this function behaves the same for the following commands: | |
| Query git for the github remote repository (based on heuristic). | |
| Unlike other funcions, this function behaves the same for the following commands: |
| return repo | ||
|
|
||
|
|
||
| def get_git_root(git_root: Path = Path()) -> Path: |
There was a problem hiding this comment.
get_git_root vs find_git_root is unclear
There was a problem hiding this comment.
Funny you mention that. I too am struggling to remember why we had this.
Was copied from the place we had those functions before.
I know it makes testing possible, but there might be a nicer way or better way to name those.
| return passed_git_root | ||
|
|
||
|
|
||
| def get_github_base_url(git_root: Path = Path()) -> str: |
There was a problem hiding this comment.
| def get_github_base_url(git_root: Path = Path()) -> str: | |
| def get_github_pages_url(git_root: Path = Path()) -> str: |
|
/consumer-test |
6116710 to
4deec1a
Compare
|
/consumer-test |
| f"{get_github_link(ws_root, n)}<>{n.file}:{n.line}" for n in needlinks | ||
| ) | ||
| def make_source_link(needlinks): | ||
| return ", ".join(f"{get_github_link(n)}<>{n.file}:{n.line}" for n in needlinks) |
There was a problem hiding this comment.
It seems that both formaters disagree on this.
Ruff (via multitool) fixed it to be like the old style.
Then our formatter fixed it again to be like the right.
There was a problem hiding this comment.
what is "our formatter"?
There was a problem hiding this comment.
the one that we run via format.fix bazel run //src:format.fix
| with open(golden_file) as f2: | ||
| json2 = json.load(f2, object_hook=needlink_test_decoder) | ||
| assert len(json1) == len(json2), ( | ||
| f"{file1}'s lenth are not the same as in the golden file lenght. " |
There was a problem hiding this comment.
Nit:
| f"{file1}'s lenth are not the same as in the golden file lenght. " | |
| f"{file1}'s lenth is not the same as the golden file. " |
| - 'bazel run' => ✅ Full workspace path | ||
| - 'bazel build' => ❌ None (sandbox isolation) | ||
| - 'direct sphinx' => ❌ None (no Bazel environment) |
There was a problem hiding this comment.
Unsure if Icons are a good idea, but I didn't want to bother to remove them.
| f"Got wrong input line from 'get_github_repo_info'. Input: {str_line}. " | ||
| + "Expected example: 'origin git@github.com:user/repo.git'" | ||
| ) | ||
| return "" |
There was a problem hiding this comment.
Nit/Next PR:
Look through this file to find out where we actually can throw exceptions etc.
As this 'empty' return might actually do more harm than good.
| assert all(c in "0123456789abcdef" for c in decoded_result) | ||
| return decoded_result | ||
| except Exception as e: | ||
| LOGGER.warning(f"Unexpected error: {git_root}", exc_info=e) |
There was a problem hiding this comment.
Nit:
| LOGGER.warning(f"Unexpected error: {git_root}", exc_info=e) | |
| LOGGER.warning(f"Unexpected error while trying to get git_hash. Exceuted in: {git_root}", exc_info=e) |
|
/consumer-test |
|
Overall it looks alright, I did check and don't see any ruff errors in the helper_lib as far as I can tell. Just so I don't introduce new linter warnnings while we try to reduce old ones. |
|
Fun.. https://github.com/eclipse-score/docs-as-code/actions/runs/16977114074/job/48128734948 |
Testing it locally, seems like a simple-ish fix. Edit: I'm guessing the consumer tests then aren't cleaning properly? Not sure where this comes from? |
71a1ec1
into
eclipse-score:main
Adding a helper_lib which adds some common used functionality. Added following functions: - find workspace root (based on bazel workspace environment var) - find git root - get & parse git remote repo - get git hash
* Followup PR to integrate nits of eclipse-score#207
Added helper lib that contains some often used functions