Skip to content

Conversation

Sanjurjo7
Copy link
Contributor

@Sanjurjo7 Sanjurjo7 commented May 17, 2016

Added to docstring for Flask.register_blueprint to clarify kwargs. Was unsure of the conventions for using :ref: but figured this fit, please let me know if there's a better way to do that.

@jeffwidman
Copy link
Contributor

You should review PEP 257, as there are a couple of docstring conventions you're missing.

@Sanjurjo7
Copy link
Contributor Author

Sanjurjo7 commented May 17, 2016

Got it. Thanks for the reference, will give it an update this evening.

@ThiefMaster
Copy link
Member

ThiefMaster commented May 20, 2016

The first word should be "Register" (command-style)

The docstring is a phrase ending in a period. It prescribes the function or method's effect as a command ("Do this", "Return that"), not as a description; e.g. don't write "Returns the pathname ...".

The code-ish parts should be wrapped in double backticks to format them as code (maybe run sphinx and build the docs locally to look at the results)

@davidism
Copy link
Member

davidism commented May 20, 2016

I appreciate the effort, but this isn't ready to merge. This doesn't read at all like the other docs. It has multiple grammar issues. It's not formatted correctly. Please review other documentation examples.

@Sanjurjo7
Copy link
Contributor Author

I'm replacing the references to the kwargs with :param kwarg: is that the bit you mean should be set apart with double backticks @ThiefMaster?

@Sanjurjo7
Copy link
Contributor Author

Sanjurjo7 commented May 20, 2016

How's that. Read up on reST in Sphinx, but I'm not sure how to run Sphinx locally, honestly.

@ThiefMaster
Copy link
Member

You just install it using pip and then use the Makefile in the docs folder to build the HTML docs by running make html

@Sanjurjo7
Copy link
Contributor Author

Okay, this format is much better, but I need to know how to get that line with the reference to the blueprints page set up correctly. What did I do wrong there?

@davidism
Copy link
Member

I'm going to merge this, then move it over to the documentation for Blueprint, and clean up a few other things. Thanks for the initiative.

@davidism davidism merged commit bdbca92 into pallets:master May 22, 2016
@Sanjurjo7
Copy link
Contributor Author

It was a good learning experience for me, thanks for all the feedback!

@Sanjurjo7 Sanjurjo7 deleted the iss1809_register_blueprint_docs branch May 22, 2016 14:15
@geusebi
Copy link
Contributor

geusebi commented Jun 11, 2016

Maybe that's a stupid question but, do register_blueprint actually handle root_path, template_folder and so on? Also why the first argument is the name of the blueprint? Shouldn't it be the actual blueprint?
This docstring looks more like a good docstring for the Blueprint constructor.
Feel free to ignore this post if I'm wrong :)
GE

@untitaker
Copy link
Contributor

This PR seems to be completely incorrect as blueprint.name is accessed further below.

@untitaker
Copy link
Contributor

@davidism Could you take a look at this again?

@davidism davidism self-assigned this Jun 14, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants