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

Indentation fixes for example code in the docs #2532

Closed
wants to merge 3 commits into from
Closed

Indentation fixes for example code in the docs #2532

wants to merge 3 commits into from

Conversation

coredumperror
Copy link
Contributor

I converted all the sample code in the docs to use 4-space indentation.

In addition, I noticed while building the docs that the Wagtail API page on Usage was triggering a ton of lexer errors. The problem was that the HTML header info at the top of the ..code-block:: json blocks is not valid json, so the lexer was failing, preventing it from syntax highlighting the json. Since the highlighting was broken anyway, I figured the simplest fix was to just use ..code-block:: text instead.

Finally, I also removed all uses of STATIC_URL (there were 3 of them), as its direct use has been unofficially deprecated in Django since the introduction of the staticfiles app. It's always more appropriate to use the {% static %} template tag, or it's equivalent pure-python function django.contrib.staticfiles.templatetags.staticfiles.static(). If your site uses a custom storage engine through the STATICFILES_STORAGE setting (e.g. S3, or some other CDN), the raw value of STATIC_URL isn't going to be the correct prefix to your static file URLs.

You get a lexer error from the document builder if you use "code-block:: json",
and the json-style highlighting ends up not being applied. So I switched it to
"code-block:: text".
Most of the samples were already 4-space indented, but a few were using 2-space,
which is both inconsistent and, when it happened with Python code samples,
incompatible with PEP8.
…c().

Concatinating with settings.STATIC_URL is no longer reccomended for creating
URLs to static resources, because it doesn't take the configured storage engine
into account. For example, a site using S3 to store its static files will need
static URLs that link out to S3, rather than relative URLs within the same
domain.

I replaced it with django.contrib.staticfiles.templatetags.staticfiles.static()
in python example code, and the {% static %} tag in template examples.
@gasman
Copy link
Collaborator

gasman commented Apr 28, 2016

Merged in 7523d7f + parents - thanks!

@coredumperror
Copy link
Contributor Author

Hmm, what does "Closed with unmerged commits. This pull request is closed, but the coredumperror:docs-indent-fix branch has unmerged commits" mean? I'm guessing that's normal and expected, but my lack of experience makes it seem scary.

@gasman
Copy link
Collaborator

gasman commented Apr 28, 2016

No worries, that's just a side effect of the way I merged the PR. If I'd merged it directly, it would have been accompanied by a 'merge' commit (like this one: 581be6a), and we try to avoid those, to keep Wagtail's commit history clean. So, before merging it in, I rebased it on top of latest master - effectively rebuilding it as if it had been written today rather than yesterday.

Unfortunately, Github isn't smart enough to recognise the resulting commits as the same ones you made here, so it thinks that I've just closed this PR without merging it...

@coredumperror
Copy link
Contributor Author

Ahh, ok. That makes sense. I also prefer to rebase, when I can, for the same reason.

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

Successfully merging this pull request may close these issues.

3 participants