👌 Improve parsing of need option lists with dynamic functions #1272
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1272 +/- ##
==========================================
- Coverage 86.87% 86.74% -0.14%
==========================================
Files 56 60 +4
Lines 6532 6908 +376
==========================================
+ Hits 5675 5992 +317
- Misses 857 916 +59
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
chrisjsewell
changed the title
sphinx_needs/api/need.py
Outdated
| # Check if tag is in needs_tags. If not raise an error. | ||
| if needs_config.tags: | ||
| for tag in tags: | ||
| needs_tags = [tag["name"] for tag in needs_config.tags] |
There was a problem hiding this comment.
tag collides with outer tag variable (not a functional problem as this is like a lambda, however still fishy.
Another thing: needs_tags is generated for every tag in tags, however is never modified, so you can move it above the for loop.
There was a problem hiding this comment.
fixed
(note I would also like to replace these exceptions with warnings, but that's another PR)
| return [] | ||
|
|
||
| if not isinstance(text, str): | ||
| assert isinstance(text, list) and all( |
There was a problem hiding this comment.
If this assertion raises it means a programming error in SN, right?
If not, just wondering, this will hard stop the build, right?
There was a problem hiding this comment.
yes it should be a programming error, so should be picked up in the test suite if failing
For need directive options of the format e.g.
:tags: a,[[func()]],bConsolidate into single conversion function and add unit test
fixes #1271