Add PUP for the use of google style docstrings #2
Conversation
pup-0002.md
Outdated
|
|
||
| 3. Following the type of `Arg` and `Return` values, there will be a colon and a single space | ||
| followed by the description. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, we also have added some django-related sections to our docs handler in 3.0, "Fields" and "Relations", used to describe fields and relations defined on models. This was determined to be needed because of the difference between fields on model classes and model instances. On model classes, the value of the field is the field class itself. On model instances, the value of the field is an instance of the field that contains its DB value, or a manager that allows querying across the relation that the field represents. As a result, it's not entirely appropriate to refer to model fields/relations as attributes (either class atrributes or instance attributes).
I propose something like:
4. `Fields` and `Relations` sections will be used when documenting fields on Django models. The `Fields` section will be used for non-related fields on Model classes, the `Relations` section will be used for related fields on Model classes.
If needed, we can reference the Django docs to help determine what fields are related:
https://docs.djangoproject.com/en/1.8/ref/models/fields/#module-django.db.models.fields.related
pup-0002.md
Outdated
|
|
||
| Some alternatives that have been discussed include: | ||
|
|
||
| 1. Tabular style to align types and descriptions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not an alternative. We can optionally implement tabular styling while still writing google-style docstrings. We may want to explicitly define if the docstrings are required to be written in a tabular style, if the docstrings can optionally be written in a tabular style, or if the docstrings must not be written in a tabular style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about s/Alternatives/Unresolved Questions/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, moving the "tabular"-ness of docstrings to Unresolved Questions is a good call.
pup-0002.md
Outdated
|
|
||
| ### Style Choices | ||
|
|
||
| Docstrings should generally follow the conventions provided in the comments section of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/generally //?
For concision, Docstrings should follow the conventions as documented, except where Pulp-specific additions or exceptions apply.
pup-0002.md
Outdated
|
|
||
| ## Summary | ||
|
|
||
| This PUP documents docstring style conventions for Pulp 3.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably add a little more here, e.g. "The PUP documents the usage of Google-style docstrings for Pulp 3+" (3+ rather than 3.0 to be clear that this doesn't apply for only 3.0, and not 3.1 or later)
Also, minor wording shifts for PR comments.
pup-0002.md
Outdated
|
|
||
| ## Motivation | ||
|
|
||
| To improve docstring readability and increase information density. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this deserves more explanation. In particular, since this would be a change from one style to another, this proposal should explain the pros and cons of making the switch as compared to staying with the current style.
Even if the discussion is limited to "The current style X would be replaced by style Y. Most current Pulp developers find style Y to be more readable than style X.", that would be very valuable to include.
Additionally, some mention of the current style in the "Alternatives" section would be appropriate.
pup-0002.md
Outdated
|
|
||
| It will take time to convert each docstrings into the new style. | ||
|
|
||
| ## Alternatives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the spirit of this section is best served if each alternative includes at least a brief discussion of pros and cons, and why the alternative is not being recommended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree in the general case, but for these specific alternatives, the pros and cons seem obvious and in the realm of personal taste. Do you have pros and cons that you would like to see added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I encourage you to state the obvious. :) I'll put some basic thoughts below.
pup-0002.md
Outdated
| ### Porting from Pulp 2.Y | ||
|
|
||
| The 3.0 development guide will be updated to state that all ported code should contain updated | ||
| docstrings. Regular expressions to ease large changes will be included there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get those regular expressions written here. I don't know what they are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced the regex should be part of this PUP. Perhaps we should consider the policy change independently of how we migrate code ported from pulp2. Also, most of the pulp3 code is either already napoleon or does not have docstrings. We should be carefully scrutinizing the code (including docstrings) ported from pulp2 anyway and don't think it will require much additional effort to migrate the docstrings (might be wishful thinking).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, I did make this comment. :) +1 to not including the regex here, but ensuring that it is included when the style guide is documented in a "porting old docstrings" section. To @jortel 's point, if all the code is converted then we can disinclude or remove that section. Either way, it is not needed in this PUP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you've written in this section I think is spot on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider rewriting last sentence for readability to: Regular expressions will be included there to ease the porting of large docstrings.
pup-0002.md
Outdated
|
|
||
| ### Leave existing old style docstrings for old code | ||
|
|
||
| We could write new style docstrings for new code only, leaving existing old style docstrings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reasons to do this:
- it would be less work
Reasons to not do this:
- it could be less readable to have multiple styles in the same code base
- it could be confusing to developers to know when they should or must update an old docstring to the new style
pup-0002.md
Outdated
|
|
||
| ### Tabular style | ||
|
|
||
| Vertical alignment of the types of objects can increase readability, but leads to bigger diffs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not actually sure what this option is describing. Could you give more detail, and provide an example?
pup-0002.md
Outdated
|
|
||
| ## Alternatives | ||
|
|
||
| ### Use default Sphinx style docstrings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it is worth specifying more explicitly that this is a current approach which we can keep.
Like "Keep current approach - use default Sphinx style docstrings". It looks more readable for me personally, though I see that you mentioned that we use it in 2.Y line in the description.
Up to you, just a suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to clarifying this is the existing style by renaming this to something like: Use Default Sphinx Style Docstrings (the Existing Style)
pup-0002.md
Outdated
| - We would not need to port old docstrings | ||
|
|
||
| Reasons not to do this: | ||
| - Porting docstrings is quick |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit in a conflict with the Drawbacks section which says "It will take time to convert each docstrings into the new style.".
Since any PUP is a proposal to do something different to current approach and you already put all the reasoning into Motivation and Drawbacks, maybe it is better to refer to those sections. What do you think?
pup-0002.md
Outdated
|
|
||
| Reasons not to do this: | ||
| - Porting docstrings is quick | ||
| - We already have mostly Google style docstrings in the new code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to my previous suggestion, if you think there are more "reasons not to do this" than specified in the Motivation, I suggest to update Motivation section then.
pup-0002.md
Outdated
| Reasons to do this: | ||
| - We would not need to port old docstrings | ||
|
|
||
| Reasons not to do this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think main reason not to keep current approach is because many people find it less readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are several reasons:
*That the Sphinx docstring style is not very readable for humans
*The existing docstring style has a lot of redundant words like param
*The existing docstring style has a lot of extra lines with each type being on its own line.
pup-0002.md
Outdated
|
|
||
|
|
||
| Reasons to do this: | ||
| - Some developers find this more readable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't necessarily find it more readable and question whether it's worth the time to do the formatting. But don't feel strongly.
pup-0002.md
Outdated
| title: Docstring Style Conventions | ||
| author: Austin Macdonald | ||
| created: 28-Mar-2017 | ||
| status: proposed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe change this Accepted so that when it's merged it's right.
|
Please add an entry in the README.md so that when it's merged it will show up |
pup-0002.md
Outdated
|
|
||
| ## Summary | ||
|
|
||
| This PUP proposes the use of Google style docstrings for Pulp 3+. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/proposes the use of Google style docstrings for Pulp 3+/adopts the Google docstring style for Pulp 3+ docstrings/
I propose ^ because when merged it will be the present tense instead of a proposal.
pup-0002.md
Outdated
|
|
||
| ### Additions/Exceptions | ||
|
|
||
| Pulp conventions that differ from the `Comments` section of the Google style guide will be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the "Comments section" become a link?
pup-0002.md
Outdated
| 1. Modules will not contain license information. Our license information is presented correctly | ||
| within the project; it is unnecessary to include it in each file. | ||
|
|
||
| 2. The type of each `Arg` value should be included after the variable name in parentheses. The type of each `Returns` value should be the first item on the line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example on the napoleon docs shows exactly this. Is this an exception?
http://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html
pup-0002.md
Outdated
|
|
||
| 2. The type of each `Arg` value should be included after the variable name in parentheses. The type of each `Returns` value should be the first item on the line. | ||
|
|
||
| 3. Following the type of `Arg` and `Return` values, there will be a colon and a single space |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example on the napoleon docs shows exactly this. Is this an exception?
http://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html
pup-0002.md
Outdated
| 3. Following the type of `Arg` and `Return` values, there will be a colon and a single space | ||
| followed by the description. Additional spaces should not be used to align types and descriptions. | ||
|
|
||
| 4. `Fields` and `Relations` sections will be used when documenting fields on Django models. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work with Napoleon? I'm not sure.
pup-0002.md
Outdated
|
|
||
| ## Motivation | ||
|
|
||
| Most current Pulp developers find Google Style docstrings to be more readable than the default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're motivated by two things:
- That the existing docstring style is not very readable for humans
- The existing docstring style has a lot of redundant words like
param - The existing docstring style has a lot of extra lines with each
typebeing on its own line. - That there is inconsistent style. Sometimes we use
@paramsometimes:paramso the current state isn't great.
This seems to focus on the solution versus a description of the problem with the current state.
pup-0002.md
Outdated
| [Google style guide](http://google.github.io/styleguide/pyguide.html?showone=Comments#Comments) | ||
| except where Pulp specific additions or exceptions apply. | ||
|
|
||
| Additionally, docstrings must be parsable by the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be moved to a section called "Requirements". We require that docstrings be parsable by Sphinx (not necessarily Napoleon).
pup-0002.md
Outdated
|
|
||
| Upon acceptance, these changes will be documented in the Pulp style guide. | ||
|
|
||
| ### Style Choices |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would call this Docstring Style
pup-0002.md
Outdated
|
|
||
| ### Use default Sphinx style docstrings | ||
|
|
||
| We could use the default Sphinx docstrings that we used in the 2.Y line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally try to remove the "we" part of it. Maybe it's not a good change but perhaps this would become:
The standard Sphinx docstring style could continue to be used. That is what is used in the 2.Y line.
pup-0002.md
Outdated
| We could use the default Sphinx docstrings that we used in the 2.Y line. | ||
|
|
||
| Reasons to do this: | ||
| - We would not need to port old docstrings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The Google Python Docstring Style may not be widely adopted and understood.
pup-0002.md
Outdated
|
|
||
| Reasons not to do this: | ||
| - Porting docstrings is quick | ||
| - We already have mostly Google style docstrings in the new code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think having some already in the code is a valid point (over time). I think writing this as if no edits were made would be good.
pup-0002.md
Outdated
| - Porting docstrings is quick | ||
| - We already have mostly Google style docstrings in the new code | ||
|
|
||
| ### Leave existing old style docstrings for old code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these headings should be capitalized (similar throughout PUP).
pup-0002.md
Outdated
| We could write new style docstrings for new code only, leaving existing old style docstrings | ||
| unchanged. | ||
|
|
||
| Reasons to do this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this become a Pros and Cons sections. I think that would be clearer that "reasons to do this" and "Reasons not to do this". Similar throughout PUP.
pup-0002.md
Outdated
|
|
||
| ### Leave existing old style docstrings for old code | ||
|
|
||
| We could write new style docstrings for new code only, leaving existing old style docstrings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style change, I usually remove the 'we' from these things:
s/We could write new style docstrings for new code only/New style docstrings could be written for new code only/.
pup-0002.md
Outdated
| Most current Pulp developers find Google Style docstrings to be more readable than the default | ||
| Sphinx style docstrings. | ||
|
|
||
| ## Details |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this section can be eliminated and the 'Style Choices' section would become a two-pound indendend instead of three.
pup-0002.md
Outdated
|
|
||
| ## Details | ||
|
|
||
| Upon acceptance, these changes will be documented in the Pulp style guide. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should get moved to a new section just above Drawbacks which would be called Putting It Into Practice which would identify several things:
- That this style would be documented in the style guide
- That the docs will be built with Napoleon as a plugin which supports this style and standard Sphinx style.
|
I think an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asmacdo Thank you so much for putting this together. I really appreciate you leading this.
I left an epic amount of comments. Address them as you see fit. I would like to look at it again so I'm leaving this note as 'requesting changes`.
09917f2
to
f22af9f
Compare
pup-0002.md
Outdated
|
|
||
| "Google style docstrings" is the preferred way to refer to the style changes outlined here. It | ||
| should be noted that the phrase "Napoleon style docstrings" was originally used. Using the term | ||
| "Napoleon" is clear because it accepts docstrings of multiple styles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean "unclear" or "not clear" ? Either way, perhaps "ambiguous" would be an improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant not clear, but I agree. This change will be on the next commit.
pup-0002.md
Outdated
| - Standard Sphinx docstrings are not very readable for humans. | ||
| - Standard Sphinx docstrings contain redundant words like `param`. | ||
| - The `type` of each param takes up its own line. | ||
| - Pulp 2.Y docstrings are inconsistent and vary in their use of `:param` and `@param`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a relic of the last time we changed styles. It is not part of our current style, so I think this point should be removed.
Pulp previously used the epydoc style: http://epydoc.sourceforge.net/
When we changed to reST, we decided to convert old code on a rolling basis, as it was worked on. As a result, some code that hasn't been touched since then is still in the old style.
At this point, converting epydoc to reST would probably be less effort than converting it to the Google style, which is why I don't think this is a "con".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point I wanted to make was that the "dont change anything" option still requires effort, but I see your point, I'm fine removing unless @bmbouter has a counter-suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean. We could make "Continue as-is" and "Convert everything to reST" separate alternatives, but that might be overkill.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to making two "Continue as-is" and "Convert everything to a consistent Sphinx style".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to remove this line because it is not really relevant to the decision to go to Google Style or stay with standard Sphinx. Also because it was really awkward to split this section :)
pup-0002.md
Outdated
|
|
||
| Pros: | ||
| - For auto-documentation, standard Sphinx and Google style docstrings are equivalent. | ||
| - This option is less work front. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/front/up-front/
pup-0002.md
Outdated
|
|
||
| ## Alternative: Tabular style | ||
|
|
||
| These guidelines (but not Google's style guide) specify the use a single space after the type of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/use a/use of a/
pup-0002.md
Outdated
| Cons: | ||
| - Standard Sphinx docstrings are not very readable for humans. | ||
| - Standard Sphinx docstrings contain redundant words like `param`. | ||
| - The `type` of each param takes up its own line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out this is a convention we fell into (I'm not sure why), but not required by the style.
http://www.sphinx-doc.org/en/stable/domains.html#info-field-lists
Example given:
:param int priority: The priority of the message, can be a number 1-5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to removing L#89 per @mhrivnaks comments
pup-0002.md
Outdated
| Pros: | ||
| - 2.Y docstrings could be moved without modification. | ||
| - The Google Python Docstring Style may not be widely adopted and understood. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think another Pro (correct me if I'm wrong) is that reST types are programmatically parsable, and the resulting docs include links to docs for corresponding types. As far as I can tell, the Google style expects type information to be described in prose, but it is not parsable.
I see that we have an item under "Exceptions" which adds types to the Google style, and matches what Napoleon shows in its examples. Does that get programmatically parsed at build time and turned into links? Is there a syntax for expressing complex types, like a list of ints? Those would be good differences to mention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you're wrong. This is not a "Pro" exclusive to the sphinx-style docstrings.
In the Google style, it's identifier (type): description, versus :param identifier: description; :type param: type. If it wasn't programatically parseable in the Google style, then napoleon would be unable to parse it. Under the hood, the Napoleon parser converts all the google-style docstrings into sphinx-style docstrings, so the behavior is the same either way. Specifically, the (type) in the identifier (type): description becomes a link to a type if that type is known during the docs build, and generally anything that sphinx can do napoleon should also be able to do.
Here's an example in the current api docs built with the google-style docstring including a type:
https://docs.pulpproject.org/en/3.0/nightly/contributing/platform_api/app/models.html#pulp.app.models.content.Content
The "notes" relation is identified as a GenericKeyValueRelation, and linked to its typedef elsewhere in the docs. Here's the corresponding docstring:
https://github.com/pulp/pulp/blob/34db0d051813ad6809da92ffd6122b02fafbd82b/app/pulp/app/models/content.py#L12-L23
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! We can leave this out then.
|
FWIW, I know I'm making several comments to ensure that merits of the current style are sufficiently represented, but please don't mistake that for opposition to the new style. |
pup-0002.md
Outdated
|
|
||
| ### Example | ||
|
|
||
| The first section is a summary, which should be restricted to a single line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the function name be added here to make it a complete docstring example?
pup-0002.md
Outdated
| Args: | ||
| arg1 (str): The argument is visible, and its type is clearly indicated. | ||
| much_longer_argument (str): Types and descriptions are not aligned. | ||
| Returns: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expected a line of whitespace between L#48 and L#49
pup-0002.md
Outdated
|
|
||
| ### Porting from Pulp 2.Y | ||
|
|
||
| The 3.0 development guide will be updated to state that all ported code should contain updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/updated/Google style docstrings/
pup-0002.md
Outdated
| The 3.0 development guide will be updated to state that all ported code should contain updated | ||
| docstrings. Regular expressions to ease large changes will be included there. | ||
|
|
||
| ### Putting It Into Practice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I suggested this be added, but a year when all of this is done it won't make any sense in the long-term document. Can this section be removed and moved to the checklist of https://pulp.plan.io/issues/2347 instead?
pup-0002.md
Outdated
|
|
||
| Pros: | ||
| - 2.Y docstrings could be moved without modification. | ||
| - The Google Python Docstring Style may not be widely adopted and understood. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Google Python Docstring Style/Google docstring style/
pup-0002.md
Outdated
|
|
||
| Docstrings should follow the conventions documented in the | ||
| [Comments section](https://google.github.io/styleguide/pyguide.html?showone=Comments#Comments) | ||
| of the Google Style Guide. Exceptions and clarifications will be explicitly stated in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/will/are written below and will/
|
I left all of my feedback. Thanks @asmacdo so much for putting this together. 👍 🥇 🍂 🚜 🌮 🎉 |
Please add your comments here or on as a reply to the announcement on pulp-dev. The comment and voting period will end on April
101217 at 9pm UTC.https://pulp.plan.io/issues/2347