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.

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

Enabling Sphinx-needs 4.1 compatibility & simplifying setup #80

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MaximilianSoerenPollak
Copy link
Contributor

@MaximilianSoerenPollak MaximilianSoerenPollak commented Nov 26, 2024

  • Upgraded to allow for compaitiblity with Sphinx-needs 4.0+
  • Changed paths to allow for better integration with other tools (like bazel).
  • Added setuptools as dependency as it wasn't there already but is used.
  • Added possibility for mutliple values in 'tr_link' targeted option
  • Enabled TestFileDirective, TestSuiteDirective, TestCaseDirective to
    have additional options from extra_links & extra_options
  • Edited tests to only test newer versions of Sphinx & Python
  • Fixed tests to account for new paths

Addresses: #78 #79


Please note:

Some of the tests do not pass the about.html is the problem, but all functionality seems to be right and present.
I'm sure there is better ways to do some of the things that makes more sense, this should be seen as 'proof of concept' and starting point.
There also is an error I could not figure out:

sphinxcontrib/test_reports/environment.py:70: RemovedInSphinx90Warning: 'sphinx.builders.html.StandaloneHTMLBuilder.css_files' is deprecated. Check CHANGES for Sphinx API modifications.

Not quite sure where that comes from, or how do fix this one. It happens with all of the environment functions, not just this one

@MaximilianSoerenPollak MaximilianSoerenPollak force-pushed the upgrade branch 3 times, most recently from 21a9a56 to 2babc7d Compare November 28, 2024 17:22
- Upgraded to allow for compaitiblity with Sphinx-needs 4.0+
- Changed paths  to allow for better integration with other tools (like bazel).
- Added setuptools as dependency as it wasn't there already but is used.
- Added possibility for mutliple values in 'tr_link' targeted option
- Enabled TestFileDirective, TestSuiteDirective, TestCaseDirective to
  have additional options from extra_links & extra_options
- Edited tests to only test newer versions of Sphinx & Python
- Fixed tests to account for new paths
@danwos
Copy link
Member

danwos commented Nov 29, 2024

Wow, this is quite a big PR and touches different fields.
Can you please split it?
Then most changes do not need to wait until all questions are answered.

I guess Sphinx-Needs 4.0 support plus setuptools should be one PR.
One PR for Path change, one for tr_link update, and one for the additional options in the directives.

This makes all our lives much easier and reviews can be done much faster.

But overall, I like all the changes and looking forward to finally merge them.

If any help is needed, just let me know.

@MaximilianSoerenPollak
Copy link
Contributor Author

Hi, yes I can split this into different parts, no problem.
This was more meant as a concept I guess. Let me work on that in the coming days and create smaller scoped ones.

I'm unsure if there is a better way to do the directives / extra_options but i think that can be discussed in the accompanying PR.

@MaximilianSoerenPollak
Copy link
Contributor Author

Currently working on splitting these up.
First one is already online, #81 , rest to follow.

@MaximilianSoerenPollak
Copy link
Contributor Author

@danwos

Just pinging you as I have split up the PR into several smaller ones. I would then close this big PR if that's okay with you and we can discuss the details inside each PR.
The PR"s are here. #81 #82 #83 #84

Just ping me inside the comments if I don't see/answer to a response from you.

Looking forward to your thoughts and inputs.

@danwos
Copy link
Member

danwos commented Dec 11, 2024

Argh sorry, for the late response.
I will take a look and try to solve your test problems with newer sphinx versions.

@MaximilianSoerenPollak
Copy link
Contributor Author

Argh sorry, for the late response. I will take a look and try to solve your test problems with newer sphinx versions.

Let me know if I can help with anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants