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

Cleanup for newest pybind11 (2.6.0) conanfile required? #3574

Closed
andioz opened this issue Nov 16, 2020 · 2 comments
Closed

Cleanup for newest pybind11 (2.6.0) conanfile required? #3574

andioz opened this issue Nov 16, 2020 · 2 comments

Comments

@andioz
Copy link
Contributor

andioz commented Nov 16, 2020

I'm glad to see the updated recipe for pybind11 2.6.0. I skimmed over the new code, and struggled starting with this line.

It will work, of course, but looks really ugly, a inner function and empty line in code before. Is it planed to refactor this recipe?

@madebr
Copy link
Contributor

madebr commented Nov 16, 2020

Empty lines are fine to mentally separate code blocks.

The use of a function is a choice by the recipe author.
He could also have used a lambda or not used a function at all.

The ugly argument is subjective and I don't agree with that.
What I agree with is that the name of the function is not good.

You can open a pr with your suggestions (or comment at #3545).
Ideal would be to read the discussion in #3305 and see what's missing/not working and add that.

@andioz
Copy link
Contributor Author

andioz commented Nov 16, 2020

You are right, I agree, I should take care of my words. What I really mean is it took me some minutes to understand this piece of code, I know the syntax for inner functions, but this is an unusual use case, I think. I followed loosely the discussions about line length, pep8, etc.

OK, I will try to use the new recipe and to figure out whats missing.

@andioz andioz closed this as completed Dec 26, 2020
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

No branches or pull requests

2 participants