Skip to content
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.

Sign up for GitHub

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

Jump to bottom

🧪 CI: Fail on sphinx-build warnings #974

Closed
wants to merge 50 commits into from
Closed

🧪 CI: Fail on sphinx-build warnings #974

wants to merge 50 commits into from

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Aug 18, 2023 •
edited
Loading

Thanks for #972 @danwos, now I can actually build the docs 😅

However, I noticed that there were still lots of warnings occuring, e.g. plantuml errors (is this what #973 is intending to fix?)

This change to the CI, should make these warnings fail the job (then obviously we need to fix those warnings)

See also #975

@danwos
Copy link
Member

danwos commented Aug 21, 2023 •
edited
Loading

Thanks for the PR, I'm waiting for a merge till we fixed all the stuff first, otherwise it will block our CI.

And somehow yes, #973 got created, because I had problems identifying the root cause of PlantUML errors, as the exact location was not printed. And because other user have also asked for it, I created a not-so-clean, but doable fix.

@danwos
Copy link
Member

danwos commented Aug 21, 2023

For PlantUML errors, notice also #977, which describes problems in needgantt

@chrisjsewell
Copy link
Member Author

I'm waring with a merge till we fixed all the stuff first, otherwise it will block our CI.

Oh indeed, thats why it marked as a draft 😄

@chrisjsewell
Copy link
Member Author

From #982, these are the actual origin of the remaining warnings:

/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/directives/needgantt.rst:20: WARNING: error while running plantuml

b'ERROR\n4\nSyntax Error?\nSome diagram description contains errors\n'
/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/directives/needgantt.rst:134: WARNING: error while running plantuml

b'ERROR\n4\nSyntax Error?\nSome diagram description contains errors\n'
/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/directives/needgantt.rst:176: WARNING: error while running plantuml

b'ERROR\n4\nSyntax Error?\nSome diagram description contains errors\n'
/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/directives/needgantt.rst:212: WARNING: error while running plantuml

b'ERROR\n4\nSyntax Error?\nSome diagram description contains errors\n'
/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/directives/needgantt.rst:250: WARNING: error while running plantuml

b'ERROR\n1\nSyntax Error?\nSome diagram description contains errors\n'
/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/directives/needgantt.rst:281: WARNING: error while running plantuml

b'ERROR\n1\nSyntax Error?\nSome diagram description contains errors\n'
/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/directives/needgantt.rst:295: WARNING: error while running plantuml

b'ERROR\n4\nSyntax Error?\nSome diagram description contains errors\n'
/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/directives/needgantt.rst:322: WARNING: error while running plantuml

b'ERROR\n4\nSyntax Error?\nSome diagram description contains errors\n'
/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/directives/needgantt.rst:366: WARNING: error while running plantuml

b'ERROR\n4\nSyntax Error?\nSome diagram description contains errors\n'
/Users/chrisjsewell/Documents/GitHub/sphinx-needs/docs/dynamic_functions.rst:63: WARNING: Filter current_need["sections"][-1]==sections[-1] not valid. Error: name 'sections' is not defined. [needs]
WARNING: Filter "Referencing and filtering needs" == section_name not valid. Error: name 'section_name' is not defined. [needs]

@chrisjsewell chrisjsewell mentioned this pull request Aug 24, 2023
Merged
@chrisjsewell
📚 DOCS: Always use headless plantuml (#983)
deef0f1 
There should be no reason why a non-headless JVM is ever required
and, without it, sphinx builds are slower and constantly switch window
focus
@chrisjsewell
🐛 Fix gantt chart rendering (#984)
7aefddc 
In plantuml V1.2022.12 (23 Oct, 2022), it became a syntax error not to
use `@startgantt`: https://plantuml.com/changes

fixes #977
@chrisjsewell
👌 Improve plantuml warnings (#982)
bf8528b 
This commit adds source origin  information to plantuml nodes, so that they can be reported by Sphinx warnings (for example if the render fails).
Note, that this change also requires sphinx-contrib/plantuml#79, to actually emit the source positions during sphinx builds
@chrisjsewell
🔧 Replace Directive with SphinxDirective (#986)
4479bee 
`SphinxDirective` provides a number of additional helper methods over the docutils class, including access to the Sphinx environment.
This commit ensures it is used for all directives.
@chrisjsewell
🧪 Add headless java to test builds (#988)
6caa3d1 
This makes it easier to run the test suite,
since it inhibits window focus switching
@chrisjsewell
🧪 Remove all make files in test build folders (#989)
a16fb72 
These are unnecessary for the running of the tests,
and this makes them more minimal
@chrisjsewell
📚 DOCS: Add intersphinx (#991)
dc815b7 
To add links in the API documentation and remove build warnings
@chrisjsewell
🧪 Remove boilerplate from test build conf.py files (#990)
bae2a35 
This removes all boilerplate/default configuration from the conf.py files,
making them more minimal to what is required for the test,
and easier to identify exactly what is being tested.
@chrisjsewell
🐛 Fix execute_func (#994)
87d326e 
Needs functions were being parsed the sphinx env, as opposed to the required sphinx app, as the first argument.
@chrisjsewell
👌 Improve logging for static file copies (#992)
8b68f20 
Only report file names
@chrisjsewell
🔧 Centralise access to sphinx-needs config (#998)
1ecb2b7 
Accessing sphinx-needs configuration via the sphinx `Config` object is difficult to work with and ensure type safety, due to its dynamic nature.

This commit moves all configuration specification to the `NeedsSphinxConfig` class, which is a thin wrapper around the sphinx `Config`, to define and provide type safe access to the sphinx-need specific configuration.

This is non-breaking change, since all configuration can still be accessed via the sphinx `Config`, (although this is now discouraged).
@chrisjsewell
🔧 Centralise sphinx env data access (and add schemas) (#987)
9ca4e23 
The sphinx-needs data stored on the Sphinx `BuildEnvironment` is currently difficult to interpret and work with,
since the dynamic nature of `BuildEnvironment` provides no type safety or static introspection.

This commit centralises access to sphinx-needs data, via `sphinx_needs.data.SphinxNeedsData`,
which is a thin wrapper around the sphinx `BuildEnvironment`,
to define and provide type safe access to the sphinx-need specific data.

`TypedDict` are used to type annotate the dictionary keys for the different data types,
and thus `sphinx_needs.data` essentially provides a schema for the data that sphinx-needs stores.

This is a non-breaking change, since all data can still be accessed via the sphinx `BuildEnvironment`,
(although this is now discouraged).
@chrisjsewell
🔧 Add strict typing for sphinx_needs.builder (#1000)
fed2965
@chrisjsewell
🔧 Make NEEDS_CONFIG type safe (#1001)
a09a539 
This commit makes `NEEDS_CONFIG` specific to the actual data that it holds, rather than just a generic data store.
@chrisjsewell
🔧 Make mypy pre-commit hook isolated (#1003)
9eb5143 
This makes it easier to run pre-commit in isolation, with no system dependencies other than pre-commit itself.
@chrisjsewell
🔧 Add strict typing for sphinx_needs.diagrams_common
c9669d0
@chrisjsewell
🔧 Add strict typing for sphinx_needs.warnings
ca029c2
@chrisjsewell
🔧 Add strict typing for sphinx_needs.environment
99b88d5
@chrisjsewell
🔧 Add strict typing for sphinx_needs.needs
1bbf9db
@chrisjsewell
🔧 Add strict typing for sphinx_needs.services.manager
afbf32a
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.needgantt
e2b277c
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.needsequence
e45bc1a
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.needlist
969363c
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.needtable
f8c4d00
@chrisjsewell
🔧 Add strict typing for sphinx_needs.roles.need_incoming
741f565
chrisjsewell and others added 24 commits August 28, 2023 17:11
@chrisjsewell
🔧 Add strict typing for sphinx_needs.roles.need_part
a19f4b0
@chrisjsewell
🔧 Add strict typing for sphinx_needs.needsfile
f3725a0
@chrisjsewell
🔧 Add strict typing for sphinx_needs.functions.common
2cd419f
@chrisjsewell
🔧 Add strict typing for sphinx_needs.external_needs
af63ab3
@chrisjsewell
🔧 Add strict typing for sphinx_needs.filter_common
6f5508f
@chrisjsewell
🔧 Add strict typing for sphinx_needs.services.github
93d2376
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.utils
d5818a4
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.needfilter
e326131
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.needflow
2e65cdd
@chrisjsewell
🔧 Fix typing of debug.measure_time
a723f9d
@chrisjsewell
🔧 Add strict typing for sphinx_needs.roles.need_refs
fb8fda9
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.needpie
9a2577d
@chrisjsewell
🔧 Disallow some Any constructs in typing
3c3445c
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.needextend
96c7506
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.needbar
4338c02
@chrisjsewell
🔧 Add strict typing for sphinx_needs.data
8994677
@chrisjsewell
🔧 Add strict typing for sphinx_needs.directives.need
ab5ea94
@chrisjsewell
🔧 Add strict typing for sphinx_needs.utils
ce9ac85
@chrisjsewell
🔧 Add strict typing for sphinx_needs.layout
146bc68
@chrisjsewell
🔀🔧 MERGE: Add strict typing for most sphinx_needs modules (#1002)
00e6afb 
This PR adds strict typing to most of the package, building on #987 and #998 to try to confine use of dynamic types to a small surface area.
This hopefully makes it easier for developers to navigate the code base, and lessens the potential for bugs arising from type mismatches.

---

There are still some open questions, particularly around `NeedsInfoType`, which

1. has some dynamic fields (such as user defined links/options) and,
2. has some fields which are only set after post-transforms (such `process_need_nodes`)
@dependabot
Bump cypress-io/github-action from 5 to 6 (#981)
9c09e84 
Bumps cypress-io/github-action from 5 to 6.
@chrisjsewell
🐛 Fix adding sections to hidden needs (#995)
321dcd5 
Previously, hidden needs would not be added to the doctree.
However, this meant that the `add_sections` transform did not add section related fields to the needs data.
This caused errors in the docs builds when filters required these fields.

The needs node is now created in the doctree, but marked with a `hidden` attribute, and these nodes are removed before rendering.
@chrisjsewell
🧪 CI: Fail on sphinx-build warnings
09f4079
@chrisjsewell
fix autodoc
0fa827d
@chrisjsewell chrisjsewell marked this pull request as ready for review August 31, 2023 12:53
@chrisjsewell
Copy link
Member Author

superceded by #1005

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chrisjsewell @danwos