sphinx-needs icon indicating copy to clipboard operation
sphinx-needs copied to clipboard

♻️ Default to warning for missing needextend ID

Open chrisjsewell opened this issue 2 years ago • 3 comments

Rather than defaulting to "strict" mode for unknown IDs in needextend directives, whereby the entire build is excepted, the default is now changed to "non-strict" mode, and this emits a warning rather than an info log.

I would suggest that actually this deprecates the use of the strict config/directive option, since the user can already halt builds on warnings using the standard sphinx-build -W flag.

Closes #1025

@arwedus this would mean you could simply remove needs_needextend_strict = True and rely on the new default warning behaviour. Make sense?

chrisjsewell avatar Nov 07 '23 10:11 chrisjsewell

I'm afraid this change destroys 1-2 use cases.

There are situations, where -W is already used, but a not found ID in needextend is okay, because the Need to extend is part of another Doc project or get imported via needimport, and this kind of separated data is not available during a local build or shall not be used, as it is too big and slows down the build too much.

So during a local build, not found needetend-IDs is fine. But during a CI build, which combines all sources, a missing ID must throw an error. And -W is used in all cases to identify problems quite quickly.

danwos avatar Nov 07 '23 11:11 danwos

And -W is used in all cases to identify problems quite quickly.

I would note, with the warning added here, it has the needs.extend type/subtype, allowing you to "fine-grain" the use of -W with https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-suppress_warnings

I'll have a think about your use case though

Perhaps one could even change the warning subtype, based on if it is strict or not, or something like that; I'm certainly more in favour of solutions that do not completely except a build

chrisjsewell avatar Nov 07 '23 12:11 chrisjsewell

@danwos: We cover that use case over our sphinx-warnings-filter extension instead of yet another config option for sphinx-needs. A [needs.extend] warning that can be configured in the sphinx warnings_filter list is imho indeed the cleanest solution.

arwedus avatar Nov 07 '23 20:11 arwedus

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 86.33%. Comparing base (42962cc) to head (dce60e1).

Files Patch % Lines
sphinx_needs/directives/needextend.py 75.00% 3 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1066      +/-   ##
==========================================
- Coverage   86.41%   86.33%   -0.09%     
==========================================
  Files          56       56              
  Lines        6531     6535       +4     
==========================================
- Hits         5644     5642       -2     
- Misses        887      893       +6     
Flag Coverage Δ
pytests 86.33% <75.00%> (-0.09%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 08 '24 10:05 codecov[bot]