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

Refs #20910 -- Replaced snippet directive with code-block. #10356

Merged
merged 1 commit into from Sep 10, 2018

Conversation

funkybob
Copy link
Contributor

Any reasons for our custom "snippet::" directive in the docs appear to have long passed into invalid obscurity.
I've replaced all uses with code-block, with a view to later being able to use :emphasize-lines: to further improve the docs.

Copy link
Member

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

Hey @funkybob.

This looks super thank you!

There was a flake8 failure, so I pushed a fix for that. (You can either squash or we can do it when merging.)

The changes look good and build without error, so 👍 as far as I can see.

Any reasons for our custom "snippet::" directive in the docs appear to have long passed into invalid obscurity.

😄 I'm not sure what those reasons were, so I'll just leave this sitting here for a while in case anyone shouts.

@funkybob
Copy link
Contributor Author

I caught @ubernostrum on IRC who believed it was to add the :filename: directive, which the Sphinx docs say is what :caption: is for, so...

@carltongibson
Copy link
Member

Thanks for the follow up! Good info. 👍🏽

@claudep
Copy link
Member

claudep commented Aug 31, 2018

Looks good, thanks!

@carltongibson
Copy link
Member

Since this is a docs fix, we'd normally back port to 2.1. I'm not sure about that though because of the code change. (Surely no-one is using this code but...)

One solution would be to split into two commits. One to just the docs using code-block and the other removing the snippet directive. Then we could back port just the one. ???

The other is to delay back porting (which we could do if/when we found conflicts occurring.)

I'll hold off for now.

@timgraham
Copy link
Member

This needs to be tested with djangoproject.com. Some changes may also be needed there, e.g. 1be2165 and the "copy to clipboard" feature. Probably it's fine to backport to 2.1 but since the older versions of the docs still use snippet, djangoproject.com will have to work with both.

@timgraham timgraham changed the title Replace snippet:: with code-block:: Refs #20910 -- Replaced snippet directive with code-block. Sep 10, 2018
@timgraham timgraham merged commit c49ea6f into django:master Sep 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants