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

[IMP] pylint_odoo: Add new check docstring-first-line-empty #16

Conversation

moylop260
Copy link
Collaborator

When odoo print a docstring just show the first line, but many times is empty

@moylop260 moylop260 force-pushed the master-oca-no-newline-docstring-moylop260 branch from bf58eb4 to b990cc3 Compare December 17, 2015 19:45
@moylop260 moylop260 self-assigned this Dec 17, 2015
self.my_method2(_('hello world'))

def my_method2(self, variable2):
'''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docstring should be with triple double-quote

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes,
broken_module have allowed break all lint cases

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in this case you are checking the beginning of the docstring with an empty line, not the correct quote. Expected errors should be then one more?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes,
We will can re-use this same method to create a new check.
broken_module has many lint issues for this matter.

@moylop260
Copy link
Collaborator Author

ping

@moylop260
Copy link
Collaborator Author

@nhomar
Could you check this pr?

@guewen
Copy link
Member

guewen commented Feb 1, 2016

I'm not sure we want to start to code the pep257 checks here. At the very least we could use https://github.com/PyCQA/pydocstyle but I'm not sure we should check pep257. I think the usage of pydocstyle could be used if wanted by a PSC but should not be a requirement. I prefer to have docstrings that do not respect pep257 (as long as we don't use them for generating documentation) than no docstrings at all, and strict rules could discourage to write them at all.

@moylop260
Copy link
Collaborator Author

Thanks for comment.

This check don't force to use docstring or check all docstyles like as pydocstyle.
This pr is much more simple.

Odoo use the method string to show docstring in log.
But even show first line.
This is a custom way to show docstring just of odoo.

This check review that if you use a docstring, you should use the first line too.

Now, we can enable this check in the plugin (to use who decide use it), but we can disable it in MQT.

What do you think?

@guewen
Copy link
Member

guewen commented Feb 2, 2016

Odoo use the method string to show docstring in log.
But even show first line.
This is a custom way to show docstring just of odoo.

When does it show the first line of the docstring in the logs?

@JulioSerna
Copy link

@guewen

this log of runbot Odoo shows this log
runbot8 odoo com runbot static build 128945 8 0 8848af logs job_20_test_all txt

and that log is about this test

as you can see in this case the log only shows the first line of docstring

@guewen
Copy link
Member

guewen commented Feb 2, 2016

this log of runbot Odoo shows this log

This is not at all a custom behavior of Odoo, this is the behavior of Python's unittest2.

@moylop260
Copy link
Collaborator Author

Thats is right.
Then this pr help to avoid log empty in this cases.

@moylop260
Copy link
Collaborator Author

@guewen
If I add a validation to check just module/tests/* folder, Could you are agree?

@moylop260
Copy link
Collaborator Author

ping

@moylop260
Copy link
Collaborator Author

There is a python generic pep for this case handling-docstring-indentation

pep 0257 docstring conventions python org

And they give us a good code:

# Since code is much more precise than words, here is an implementation of the algorithm:

def trim(docstring):
    if not docstring:
        return ''
    # Convert tabs to spaces (following the normal Python rules)
    # and split into a list of lines:
    lines = docstring.expandtabs().splitlines()
    # Determine minimum indentation (first line doesn't count):
    indent = sys.maxint
    for line in lines[1:]:
        stripped = line.lstrip()
        if stripped:
            indent = min(indent, len(line) - len(stripped))
    # Remove indentation (first line is special):
    trimmed = [lines[0].strip()]
    if indent < sys.maxint:
        for line in lines[1:]:
            trimmed.append(line[indent:].rstrip())
    # Strip off trailing and leading blank lines:
    while trimmed and not trimmed[-1]:
        trimmed.pop()
    while trimmed and not trimmed[0]:
        trimmed.pop(0)
    # Return a single string:
    return '\n'.join(trimmed)

Then I will close this one and we will work in follow issue: pylint-dev/pylint#868

@moylop260 moylop260 closed this Apr 7, 2016
@moylop260 moylop260 deleted the master-oca-no-newline-docstring-moylop260 branch June 30, 2016 22:09
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.

None yet

4 participants