Upgrade sphinx-needs to 6.3.0#361
Conversation
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 |
ubmarco
left a comment
There was a problem hiding this comment.
LGTM, let's update SN in a first step, then do the schema validation translation.
Which Sphinx version is resolved now? We should also set a minimum version, to also fix deprecations and use latest features. Can also do that in another PR.
sphinx==8.2.3, the same than before the sphinx-needs update. I committed the minimum version directly in requirements.in. |
|
This looks way less complex than expected for that update 👍
|
Yes, at least the score doc built locally with that update.
I just applied, waiting for approval.
In fact I found the script and had some problem running it. Seems like ruff / actionlint are not found. Maybe I didn't setup my devcontainer correctly? (ide_support ran successfully) |
276c5ce to
fd6645c
Compare
| kwargs.setdefault("id", "test_need") | ||
| kwargs.setdefault("type", "requirement") | ||
| kwargs.setdefault("title", "") | ||
| kwargs.setdefault("status", None) | ||
| kwargs.setdefault("tags", []) | ||
| kwargs.setdefault("collapse", False) | ||
| kwargs.setdefault("hide", False) | ||
| kwargs.setdefault("layout", None) | ||
| kwargs.setdefault("style", None) | ||
| kwargs.setdefault("external_css", "") | ||
| kwargs.setdefault("type_name", "") | ||
| kwargs.setdefault("type_prefix", "") | ||
| kwargs.setdefault("type_color", "") | ||
| kwargs.setdefault("type_style", "") | ||
| kwargs.setdefault("constraints", []) | ||
| kwargs.setdefault("arch", {}) | ||
| kwargs.setdefault("sections", ()) | ||
| kwargs.setdefault("signature", None) | ||
| kwargs.setdefault("has_dead_links", False) | ||
| kwargs.setdefault("has_forbidden_dead_links", False) |
There was a problem hiding this comment.
Seems a bit excessive.
Do we really need to declare all the defaults again? Can't we take the ones from sphinx-needs internally?
There was a problem hiding this comment.
These are already the internal attributes, see: https://github.com/useblocks/sphinx-needs/blob/6371b3a0f13f25e85a2c08c568d0b1eba65e060d/sphinx_needs/data.py#L452
Do you think we need to reduce this to a minimum? What required subset would you suggest?
There was a problem hiding this comment.
intentional change? Is that new?
There was a problem hiding this comment.
Sorry, I didn't see you're commit earlier. Now that the PR is merge, it's all good, right? Or did I miss something?
There was a problem hiding this comment.
ah this includes all change on main branch somehow! Can you try rebasing or whatever to clean that up?
There was a problem hiding this comment.
Sorry, I didn't see you're commit earlier. Now that the PR is merge, it's all good, right? Or did I miss something?
Add support for new options (is_import, constraints) introduced in 6.3.0 and remove the plantuml workaround that was only needed for older versions.
…test_source_code_link_integration
fd6645c to
a7b7354
Compare
|
rebased |
| source_content_keys = { | ||
| "docname", | ||
| "lineno", | ||
| "lineno_content", | ||
| "external_url", | ||
| "is_import", | ||
| "is_external", | ||
| "doctype", | ||
| "content", | ||
| "pre_content", | ||
| "post_content", | ||
| } |
There was a problem hiding this comment.
Can you explain this?
There was a problem hiding this comment.
These are the internally provided keys by sphinx needs for the NeedItemSourceUnknown and NeedsContent classes, they are filtered out in the next loop.
| actual_links: set[str] = ( | ||
| set(actual_source_code_link.split(", ")) | ||
| if actual_source_code_link |
There was a problem hiding this comment.
This will always be true as it has a default value:
treq_info.get("source_code_link", "no source link")
There was a problem hiding this comment.
You're right.
I can prepare a fix that aligns the pattern to the check on expected_code_link.
There was a problem hiding this comment.
Ye this can happen in a future PR.
THanks again for this work. Great work
5ce4077
into
eclipse-score:main
* feat: upgrade sphinx-needs to 6.3.0 Add support for new options (is_import, constraints) introduced in 6.3.0 and remove the plantuml workaround that was only needed for older versions. * feat: added a minimum version requirement for sphinx * refactor: replace NeedsInfoType with NeedItem across multiple files * refactor: remove unused link keys from need function * refactor: reorganize imports in test and source code linker modules * refactor: enhance type hints for better clarity in check_options and test_source_code_link_integration * refactor: improve type casting and validation in _get_normalized function

Upgrade sphinx-needs to 6.3.0
📌 Description
Upgrade sphinx-needs to version 6.3.0
Add support for new options (is_import, constraints) introduced in 6.3.0 and remove the plantuml workaround that was only needed for older versions.
🚨 Impact Analysis
✅ Checklist