Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Allow docstring in function definition #126
Allow docstring in function definition #126
Changes from 3 commits
e051812
66c9f74
398b660
713be26
ffa04c6
16b564a
d3eeb75
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Thoughts on removing these empty fields from the tests? Would make it easier to read imo / remove some bloat
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 using
ast.parse
as in some other tests?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've gone with this format. Unfortunately due to the nature of multiline strings it seems like thedef
can't have any preceding indentation, which makes it a bit awkward to read, but still better than before.There can't be any space between the last non-whitespace character and the closing"""
.I decided to opt against the starting
\
because it doesn't actually add anything in terms of the test, and I felt that it's better to keep it simple for readability.edit: could probably add this function into
test_utils
if it's commonly used, but since it's only two cases that can probably get held offThere 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 is not recommended according to the DAMP principle. It is somewhat desirable to write self-contained tests:
https://abseil.io/resources/swe-book/html/ch12.html#tests_and_code_sharing_dampcomma_not_dr