-
Notifications
You must be signed in to change notification settings - Fork 11
new integration tests for acme http-01 and website change #45
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
Conversation
|
Big thanks for enabling these. |
|
@sciros I thought the original lines looked a bit better, but not by much. I would prefer consistency, so I just ran black formatter on the code files. If the integration tests look good to you I would like to proceed. |
|
Yeah.. I'd have wrapped them in fmt: off/on because this to me is a clear case of rigid formatting rules absolutely butchering readability. Trying to tell whether we're missing a specific test case while having to scroll through a couple of screens' of lines is exactly where rigid, naive formatters completely fall over and should be discarded. I believe you should revert this change. |
sciros
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert application of Black formatter to parametrize lines.
|
Personally I did not object to the formatting as strongly. These test files can get crazy large and I was already primarily using search to get around. Regardless, I agree the original line style was better and I wrapped with fmt on and fmt off. Then I ran the formatter on the rest of the file. @sciros let me know your thoughts. |
I think this is a good compromise. Thanks for making the change. Honestly, for now I am regretting a little bit the incorporation of the Black formatter, given how we've not had any issues with formatting AT ALL up to this point, except for ironically just ones that Black introduced, and having to toss in format on/off switches because of Black's limitations (can't specify statements to ignore, can't apply formatting to parts of files) doesn't exactly contribute to "beautiful Python," but I am hoping that this maybe heads off any potential issues with this being OSS. Probably I'll take this as a lesson learned to not include it in future Python efforts unless it somehow becomes necessary, and instead tell people to just follow PEP8 (https://pep8.org/) guidelines. |
No description provided.