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

Custom methods must be protected #127

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

uilianries
Copy link
Member

@uilianries uilianries commented Nov 1, 2019

  • Any method declared as public, must be provided from ConanFile
  • Any custom method must be declared as protected
  • Any private method is not be accepted

closes #126

Wiki:

#KB-H036: "CUSTOM METHODS"

When a new method have to be added to the recipe, and it is not a default one provided by ConanFile, it must added as protected:

class Recipe(ConanFile):
    def configure(self): # default methods are public
        pass

    def _configure_cmake(self): # custom methods are protected
        pass

    def __private_routine(self): # private should not be used. Use protected instead.
	    pass

mock = ConanFile(conanfile.output, None)
valid_attrs = get_methods(mock)
current_attrs = get_methods(conanfile)
invalid_attrs = re.findall(r"def (__.*[^_])\s?\(", conanfile_content, re.MULTILINE)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get it, you query valid attrs and current attrs, why query invalid attrs with different method?
isn't result the same as current_attrs - valid_attrs?

Copy link
Member Author

Choose a reason for hiding this comment

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

private methods are not filtered by dir method. e.g. def __foo(self):

""")
tools.save('conanfile.py', content=conanfile)
output = self.conan(['create', '.', 'name/version@user/test'])
self.assertIn("ERROR: [CUSTOM METHODS (KB-H036)] Custom methods must be declared as "
Copy link
Contributor

Choose a reason for hiding this comment

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

might be too strict, as we may define some magic methods (__str__, __eq__, __new__, __init__, __enter__, __dir__, etc)?

Copy link
Member Author

Choose a reason for hiding this comment

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

but it's the exception you know. It sounds like cccl and other few recipes. We know that is not a regular case, but new contributors can use private instead of protected when writing a recipe, for instance.

@SSE4
Copy link
Contributor

SSE4 commented Nov 7, 2019

@uilianries also, I wonder, why do we forbid to use private members? is it somehow bad/harmful?

@uilianries
Copy link
Member Author

@SSE4 private attributes are not bad, but how many times we needed them for a recipe? Maybe twice all this time, only for workarounds and helpers, which are not the regular flow.

@uilianries uilianries requested a review from danimtb April 28, 2020 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom method must be protected
2 participants