Closed codesoap closed 6 years ago
Darn, I had run the tests locally, but didn't see the .pylintrc .
I couldn't quickly come up with a fix for the problematic lines. I'll keep trying and hopefully be able to add another commit in the next days.
OK, looks like the PR is ready to be reviewed, now.
I'm sorry, but I disagree that this improves readability. Instead of reading the file from top-to-bottom, you now need to jump back and forth between a bunch of nested functions. The functions themselves don't provide any value over the block comments that they're replacing. Everything is already contained to a single file and there's not code duplication. Sometimes simple is better than complex.
I admit that this is a personal preference and people have different coding styles. There's nothing objectively wrong with this PR, the code looks good and it passes the tests. But it's not the direction that I want to go in.
Alright, thank you for explaining your standpoint. I can accept that you have a different taste in code.
I don't intend to change your mind about the PR, but in case you are interested in what benefits I see:
My taste in code is strongly influenced by the book "Clean Code".
I know you explicitly asked people not to make style-only pull requests. I know it's a big commit.
But I think it improves readability considerably and I didn't change anything in the logic.