Skip to content

Conversation

@uvchik
Copy link
Member

@uvchik uvchik commented Dec 9, 2019

Fixes #81 .

Test stickler-ci for the windpowerlib.

@uvchik uvchik self-assigned this Dec 9, 2019
@uvchik uvchik added this to the v0.2.1 milestone Dec 9, 2019
@vezeli
Copy link
Contributor

vezeli commented Dec 9, 2019

@uvchik If this gets merged and the contributors are supposed to run Black before submitting a PR than adding information in "Contributing" reflecting this should be needed? Also adding it now seems OK.

@uvchik
Copy link
Member Author

uvchik commented Dec 9, 2019

If it works as intended you do not have to run Black before submitting a PR. At the moment you have to follow the pep rules anyway. Stickler-ci will check that and Black may fix it automatically but at the moment it does not seem to work. Maybe I have to merge it to get it work.

@vezeli
Copy link
Contributor

vezeli commented Dec 9, 2019

Oh nice, that is actually a really neat service to have! When you get it working I found some information here and here that accounts for the incompatibility between flake8 and black and makes them work together better.

@uvchik uvchik requested review from SabineHaas and birgits and removed request for birgits December 12, 2019 15:32
@uvchik
Copy link
Member Author

uvchik commented Dec 12, 2019

Now it works. According to @gnn the black style might not be "everybody's darling" but is well documented and every decision is explained in the black documentation. I personally do not like everything but as it makes things easier I would vote for using it.

@SabineHaas
Copy link
Member

Thanks for implementing this @uvchik!
I have to admit that I don't like the open brackets being moved to the next line.. Is there any way to change settings?

Anyways, I really appreciate that we then have a consistent style (no matter the brackets ;))
:)

@uvchik
Copy link
Member Author

uvchik commented Jan 7, 2020

I have to admit that I don't like the open brackets being moved to the next line.. Is there any way to change settings?

I do not like them either 😏

But I would not change it, because of two reasons.

  1. If you start with exceptions you could discuss any style element. Why not use 100 instead of 80 characters and so on. To avoid such discussions I would use the full package without exceptions (de gustibus non est disputandum).

  2. Black really explains everything. In this case it is used to make diff-files easier to read (->https://black.readthedocs.io/en/stable/the_black_code_style.html#how-black-wraps-lines)

I would like both of you (@birgits, @SabineHaas) to approve this issue. @vezeli already reacted with the 👍 emoji.

Copy link
Member

@SabineHaas SabineHaas left a comment

Choose a reason for hiding this comment

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

Great! Thanks a lot :)

Copy link
Member

@birgits birgits left a comment

Choose a reason for hiding this comment

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

Thanks @uvchik for implementing this!

@uvchik uvchik merged commit e20310b into dev Jan 8, 2020
@uvchik uvchik deleted the features/test-stickler-ci branch January 8, 2020 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shall we use structure and pep checker

6 participants