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

Do Not Merge: Use Black in Moulinette #220

Open
wants to merge 1 commit into
base: stretch-unstable
from

Conversation

@decentral1se
Copy link
Member

commented Sep 1, 2019

Because manually formatting things sucks.

https://github.com/psf/black

This will probably conflict with every other PR ☁️

To run it, you need a Python 3 installed (see included docs) and then just run:

$ tox -e format  # have black format things
$ tox -e format-check  # only check if source needs formatting

If this is well received, we can think about it over at at yunohost/yunohost. This comes after some discussion over at YunoHost/yunohost#787.

@decentral1se decentral1se force-pushed the decentral1se:black-and-moulinette branch 2 times, most recently from 08380ce to 7295e30 Sep 1, 2019

@Psycojoker

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

This will probably conflict with every other PR cloud

Except if we run it on other PRs?

@Psycojoker

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

We should probably put some stuff in place to automatically run black on every code merge also.

@decentral1se

This comment has been minimized.

Copy link
Member Author

commented Sep 2, 2019

This will probably conflict with every other PR cloud

Except if we run it on other PRs?

Yep, but I always prefer to not make conflicts for others ;) But running tox -e format isn't bad.

We should probably put some stuff in place to automatically run black on every code merge also.

Travis CI will run on every PR and every merge into the stretch-unstable branch. Do you mean to run Black and have it format the source and then commit it? That sounds nice but like I saw in 7295e30, it still needs some supervision.

@decentral1se decentral1se referenced this pull request Sep 2, 2019
0 of 4 tasks complete

@decentral1se decentral1se force-pushed the decentral1se:black-and-moulinette branch 2 times, most recently from 194df2a to a126b8a Sep 10, 2019

@decentral1se

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2019

What do you think @Psycojoker, shall we merge or leave it for now?

@decentral1se decentral1se force-pushed the decentral1se:black-and-moulinette branch from a126b8a to 48f2aac Sep 13, 2019

@alexAubin

This comment has been minimized.

Copy link
Member

commented Sep 13, 2019

Ugh if we could merge #216 first that would help to not have a shitload of conflicts to handle :s

@decentral1se decentral1se force-pushed the decentral1se:black-and-moulinette branch from 48f2aac to c4d8c8c Sep 13, 2019

@decentral1se

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2019

Ugh if we could merge #216 first that would help to not have a shitload of conflicts to handle :s

Sure thing, I'll wait until then 👍

@decentral1se decentral1se force-pushed the decentral1se:black-and-moulinette branch from c4d8c8c to 730f7d1 Sep 13, 2019

@decentral1se decentral1se changed the title Use Black in Moulinette Do Not Merge: Use Black in Moulinette Sep 13, 2019

@Psycojoker

This comment has been minimized.

Copy link
Member

commented Sep 14, 2019

Ugh if we could merge #216 first that would help to not have a shitload of conflicts to handle :s

I'm really wondering if just applying black on #216 wouldn't just solve all conflicts tbh

@Psycojoker

This comment has been minimized.

Copy link
Member

commented Sep 14, 2019

(but that needs to be tested but that would really solve problems)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.