Updated guidelines for single line docstrings

[clintonb]

The guideline now aligns with PEP 257. There is no obvious reason for deviating.

[rlucioni]

👍

[pdesjardins]

👍

[dan-f]

👍 - could we specify whether or not we allow 'buffer' spaces? e.g. """A single-line docstring""" vs """ A single-line docstring """?

My preference is the first option, since it follows how we don't pad function calls/dicts/etc with spaces.

[rlucioni]

I agree with @dan-f, my preference is no whitespace.

[doctoryes]

I prefer consistency between one- and multi-line comments so 👎 . But - the PEP is stacked against me and I'm flexible.

[stvstnfrd]

Redacted: ignore: premature submission

[stvstnfrd]

I'm with @doctoryes for consistency.
By consistently using multiline docstring, you reduce the size of the
diff, in the event that documentation is ever expanded [1].

I doubt you guys worry about how code diffs in the same way we do;
it's a constant pain living down-wind from edX.

Imagine a code block like:

def get_users():
    """
    Get teh users
    """

But then at our fork, we need to explain additional
assumptions/considerations, so we augment the docs:

def get_users():
    """
    Get teh users

    At Stanford, we must also filter out all DirectAccess users to
    exclude anonymous access from the reports.
    """

If everyone always uses multiline, the diff is clean and less likely to
conflict.

def get_users():
    """
    Get teh users
+
+     At Stanford, we must also filter out all DirectAccess users to
+     exclude anonymous access from the reports.
    """

So if upstream eventually decides to fix the initial typo and/or add
more context, we won't have conflicts when we merge.

def get_users():
    """
-     Get teh users
+     Get the users

    At Stanford, we must also filter out all DirectAccess users to
    exclude anonymous access from the reports.
    """

On the other hand, if we need to edit your single-line docstring, we're
guaranteed to have conflicts if we both edit the docs.

def get_users():
<<<<<<< US
    Get teh users

    At Stanford, we must also filter out all DirectAccess users to
    exclude anonymous access from the reports.
    """
=======
    """Get the users"""
>>>>>>> THEM

You may consider that trivial, but when we're resolving dozens and
dozens of those every time we merge, it quickly becomes a real pain.
And of course, the more conflicts we have to resolve, the more chance
there is we'll mistakenly introduce bugs during conflict resolution;
we've been bitten by this before.

Perhaps there's a larger discussion to be had w/r/t "the style of the
diff", but many of the ways in which edX writes code makes it more
difficult than it has to be to integrate forked code. Given that, while
I consider PEP8/257/etc. to be a good baseline, I would like to see us
enforce stricter guidelines when/where it makes the code easier to work
with.

[1] Everyone is always positive that this one-liner will always just
be a single line... that often doesn't hold.

[dan-f]

Thats a super useful (and well written!) example @stvstnfrd. I think we should prioritize our community's ability to work off of our platform above a PEP.

[nasthagiri]

@clintonb I second the comment from @stvstnfrd and prefer keeping with the multi-line comment standard as it is best for code reviews and diffs. As an open source platform, I agree that we should weigh the convenience of code reviewers over PEP8 standards.
So 👎 .

[dan-f]

Is there any interest in updating the docs to explicitly call out the issue that @stvstnfrd keeps on running into? It seems as though people are ignoring this documentation, and it might be helpful motivation.

I can do it if there's sufficient interest.

[rlucioni]

@dan-f I think documenting the issue @stvstnfrd explained would be a great idea. If we're going to deviate from PEP 257, it would be good to explain why.