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

Expose metadata to sphinx #6

Merged
merged 1 commit into from
Jun 28, 2023
Merged

Expose metadata to sphinx #6

merged 1 commit into from
Jun 28, 2023

Conversation

pkulev
Copy link
Contributor

@pkulev pkulev commented Jun 28, 2023

Hello! First of all thank you for the wonderful package!
I enjoy using it for writing my documentation ❤️.

I use sphinx v5.3.0 with parallel builds (-j auto) and it shows the following warning:

WARNING: the sphinx_code_tabs extension does not declare if it is safe for parallel reading, assuming it isn't - please ask the extension author to check and make it explicit
WARNING: doing serial read

I tinkered through the code and thought that it's pretty safe for parallel reads.
I looked into the docs and then made changes for passing metadata info to the sphinx application.

This PR also contains some style fixes according to common practices (pep8, black, isort), but if you don't want them - I will drop those style-related commits.

@coldfix
Copy link
Owner

coldfix commented Jun 28, 2023

Thanks!

This PR also contains some style fixes according to common practices (pep8, black, isort), but if you don't want them - I will drop those style-related commits.

Could you drop that please for this PR. That should go in a separate PR, where it can be discussed independently. There are also some changes for which I don't see a clear reason immediately (e.g. changing lists to sets).

You don't need to increase the version number either. I will do that in the release commit :)

@coldfix
Copy link
Owner

coldfix commented Jun 28, 2023

Also, while I think the idea of on autoformatter is good in general, I'm not a big fan of black specifically, as it often leads to very suboptimal formatting (IMO). I've seen many instances of code which is just not very readable anymore after what black does to it.

@pkulev
Copy link
Contributor Author

pkulev commented Jun 28, 2023

I understand your point.
I've already used to autoformatting and it makes great sense for me on large code bases when I have to only review logic and pass formatting and style-related things to the machine 😄

I've dropped those commits.

@coldfix
Copy link
Owner

coldfix commented Jun 28, 2023

Thanks, so much easier to see the relevant change now!

@coldfix coldfix merged commit e659014 into coldfix:main Jun 28, 2023
coldfix added a commit that referenced this pull request Jun 28, 2023
- declare extension as safe for parallel reads (#6)
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