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

app.add_stylesheet() is gone and replaced by app.add_css_file #9683

Closed
thomasgoirand opened this issue Sep 28, 2021 · 4 comments
Closed

app.add_stylesheet() is gone and replaced by app.add_css_file #9683

thomasgoirand opened this issue Sep 28, 2021 · 4 comments
Milestone

Comments

@thomasgoirand
Copy link

@thomasgoirand thomasgoirand commented Sep 28, 2021

Describe the bug

This breaks a LOT of projects. Please revert the removal.

As a package maintainer in Debian that is in charge of 500+ packages, this is not fun. There's no reason to deprecate and remove methods just because you think it's nicer. Please behave yourself. Nobody does that on the Linux kernel since its inception. It is unacceptable that breaking the world feels OK-ish to python developers, and shows a gave lack of seriousness.

How to Reproduce

For example, try to build pyroute2 with Sphinx 4.

Expected behavior

No response

Your project

Debian packaging of MANY packages

Screenshots

No response

OS

debian

Python version

3.9

Sphinx version

4.2.0

Sphinx extensions

No response

Extra tools

No response

Additional context

No response

@astrojuanlu
Copy link
Contributor

@astrojuanlu astrojuanlu commented Sep 28, 2021

I'm not an Sphinx maintainer but I'm going to leave my 2 cents.

app.add_stylesheet has been deprecated since Sphinx 1.8.0b1:

sphinx/CHANGES

Line 2657 in f34eb49

* ``app.add_stylesheet()`` is deprecated

which was tagged 3 years ago: https://github.com/sphinx-doc/sphinx/tree/v1.8.0b1

I know that many Sphinx extensions don't have a lot of activity, and I see how they could have stopped working without their maintainers noticing. But it would be more helpful to list which packages are broken by this removal, among the ones you maintain. Or, give more context on why Debian chose to ship Sphinx 4 rather than, say, latest Sphinx 3.

But instead of being helpful, you chose to write this:

As a package maintainer in Debian that is in charge of 500+ packages, this is not fun. There's no reason to deprecate and remove methods just because you think it's nicer. Please behave yourself. Nobody does that on the Linux kernel since its inception. It is unacceptable that breaking the world feels OK-ish to python developers, and shows a gave lack of seriousness.

Which I think goes against pretty much all of the Code of Conduct:

  • Be friendly and patient.
  • Be welcoming.
  • Be considerate.
  • Be respectful.
  • Be careful in the words that you choose.
  • When we disagree, try to understand why.

Except that, well, at least it doesn't contain explicit insults or harassment. But I think that bar is pretty low.

@thomasgoirand
Copy link
Author

@thomasgoirand thomasgoirand commented Sep 29, 2021

Hi.

Please re-read what I wrote, and you'll realize that there's absolutely nothing insulting. I'm sorry if you took it the wrong way. My message was: never break any of you reverse-dependency, and don't take this lightly, it is very important and serious. Sphinx is very high profile because used everywhere, and it'd be nice if you realized it. There's nothing un-respectful when saying this.

For searching occurrences, it's very easy:
https://codesearch.debian.net/search?q=app.add_stylesheet&literal=1

The fact that you're saying you deprecated your own API 3 years ago is irrelevant. Some packages are maintained for much longer that this, with no upstream activity. Also, Debian cannot "choose" a specific version of Sphinx. We must upgrade, so that we're as close as possible to upstream in case there's some security issues. Also, some packages imposes the latest version, otherwise they wouldn't build. That's exactly what happened with Sphinx 4, and it will happen again with next versions. The only thing we can do, is embrace the latest version and fix whatever package broke.

I hope this helps.

@johnmaconline
Copy link

@johnmaconline johnmaconline commented Sep 30, 2021

+1 on reverting. Open/Closed Principle is a big deal for a project like this, regardless of how long the community has been warned. For example, the current sphinx containers on docker hub can't build documents correctly right now in the same way one does on readthedocs. They don't get the stylesheet reference in the HTML output.

Or, to be clear, what worked locally (with and without containers) and on readthedocs prior to this deprecation no longer works.

@tk0miya
Copy link
Member

@tk0miya tk0miya commented Oct 3, 2021

I'm okay to revert the API temporarily (for example, 2years). But I can't agree on the idea that Sphinx's API should never change the API. I believe it prevents our improvements.

In this topic, using the add_stylesheet() API in each project is a very hacky solution. It was replaced by the configuration html_css_files since Sphinx-1.8. So it's better to use it instead of hacky code.

@tk0miya tk0miya added the api label Oct 3, 2021
@tk0miya tk0miya added this to the 4.3.0 milestone Oct 3, 2021
@tk0miya tk0miya closed this in 1fbca49 Oct 9, 2021
tk0miya added a commit that referenced this issue Oct 9, 2021
Close #9683: Revert the removal of ``add_stylesheet()`` API
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants