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

Make setters private #69

Closed
wants to merge 2 commits into from
Closed

Make setters private #69

wants to merge 2 commits into from

Conversation

@adiroiban
Copy link

@adiroiban adiroiban commented Apr 14, 2014

Problem

pep257 complains that setters (and mabye deleters) have no docstring.

Here is an example

    @property
    def enabled(self):
        """
        See: `IEntityConfiguration`.
        """
        return self._proxy.getBoolean(self._section_name, 'enabled')

    @enabled.setter
    def enabled(self, value):
        return self._proxy.setBoolean(
            self._section_name, 'enabled', value)

Documentation tools should ignore setters and deletes docstring when creating api docs.
Also, setters and deleters are pretty standard and they will only have a boilerplate text which will not say anything new.

Changes

Updated the parser to keep a record for decorators.

In Method.is_private i check the list of decorators and if decorators start with same name then it is marked private.

I have removed unused / broken tests from test_pep257 as this is just hoarding of junk. http://blog.8thlight.com/uncle-bob/2014/04/03/Code-Hoarders.html

Added test for new code.

How to test

Check that changed make sense

py.test test_pep257.py -v

Let me know if you find this useful.

Thanks!

@sigmavirus24
Copy link
Member

@sigmavirus24 sigmavirus24 commented Apr 14, 2014

Loading

@adiroiban
Copy link
Author

@adiroiban adiroiban commented Apr 14, 2014

Master is also failing and is not my fault :( : https://travis-ci.org/GreenSteam/pep257/builds/22006902

Please see command used for running my tests.

Loading

@adiroiban
Copy link
Author

@adiroiban adiroiban commented Apr 14, 2014

I now see that pep8 is failing. I will udpate my code ... is ugly to work with a master branch which is not clean.

Loading

@Nurdok
Copy link
Member

@Nurdok Nurdok commented Nov 18, 2014

@adiroiban the master branch is now clean. Can you pull and merge it with your code so I can see if your tests are failing or not?

Loading

@keleshev keleshev self-assigned this Nov 18, 2014
@adiroiban
Copy link
Author

@adiroiban adiroiban commented Nov 18, 2014

Hi. I don't have time right now to continue working on this but feel free to fork the branch.

Loading

glennmatthews added a commit to glennmatthews/pep257 that referenced this issue Feb 18, 2015
Nurdok added a commit that referenced this issue Mar 25, 2015
Make setters private (rework of #69)
@glennmatthews
Copy link
Contributor

@glennmatthews glennmatthews commented Jul 9, 2015

This can probably be closed now that #107 is merged.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants