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

Fix: Empty string enum rendering + Add configurable amount of choices to display #42

Merged
merged 1 commit into from Jul 2, 2023
Merged

Conversation

insspb
Copy link
Contributor

@insspb insspb commented Jun 13, 2023

This PR will fix text choices rendering problem, from issue #41 and also it add completely clear approach to configure amount of choices to render (with sphinx option).

fixes #41

@timoludwig Please merge it

@insspb insspb changed the title Fix empty string + Add configurable amount of choices to display Fix: Empty string enum rendering + Add configurable amount of choices to display Jun 13, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2023

Codecov Report

Merging #42 (3324733) into main (db7325a) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 3324733 differs from pull request most recent head eb13e40. Consider uploading reports for the commit eb13e40 to get more accurate results

@@            Coverage Diff            @@
##              main       #42   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          314       309    -5     
=========================================
- Hits           314       309    -5     
Impacted Files Coverage Δ
sphinxcontrib_django/docstrings/config.py 100.00% <ø> (ø)
sphinxcontrib_django/docstrings/__init__.py 100.00% <100.00%> (ø)
sphinxcontrib_django/docstrings/attributes.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, looks good to me! 🎉

I'd just keep the default value of the option in sync with the tests to make sure the tests really cover the steps below and above the actual limit.

tests/roots/test-docstrings/dummy_django_app/models.py Outdated Show resolved Hide resolved
@insspb
Copy link
Contributor Author

insspb commented Jun 30, 2023

@timoludwig Any news?

Co-authored-by: Timo Ludwig <ti.ludwig@web.de>
Copy link
Collaborator

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your changes! 👍

I added a changelog entry and one additional test for the empty label.

@timobrembeck timobrembeck merged commit 9d5be4e into sphinx-doc:main Jul 2, 2023
31 checks passed
ntouran added a commit to ntouran/sphinxcontrib-django that referenced this pull request Aug 16, 2023
Fixes sphinx-doc#42.

The tests intended to capture the issue but actually
don't trigger the real issue in the test scenario,
so they may not be appropriate. I tried making
a 2nd app to see if the deferred issue would show
up but it does not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants