Skip to content
This repository has been archived by the owner on Sep 21, 2023. It is now read-only.

Black #129

Closed
wants to merge 2 commits into from
Closed

Black #129

wants to merge 2 commits into from

Conversation

itsthejoker
Copy link
Member

#127

This is a "hey, let's see what it looks like and if we want to go this route" kind of PR. If we don't like it, we can close it, no big deal.

This is the result of running:

black setup.py && black tor

in the root directory, using the default configurations for Black. Honestly I do kind of like how it turned out, but I'm curious as to your thoughts.

@itsthejoker itsthejoker requested a review from a team as a code owner June 27, 2018 21:56
@ghost ghost assigned itsthejoker Jun 27, 2018
@ghost ghost added the in progress label Jun 27, 2018
random.choice(config.no_gifs)
)
)

return

logging.debug(
f'Now executing command {requested_command},'
f' by {reply.author.name}.'
f"Now executing command {requested_command}," f" by {reply.author.name}."
Copy link
Member

Choose a reason for hiding this comment

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

This right here is an error, as far as the python interpreter is concerned.

I noticed the same when I was running black on the celery rewrite code, noticing it wasn't perfect at rewriting the multi-line string stuff or understanding f-strings.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. Looks like this is a known bug. psf/black#369

Copy link
Member

Choose a reason for hiding this comment

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

Looks like even that is a duplicate of a long(ish)-standing bug: psf/black#26

Copy link
Member

@TheLonelyGhost TheLonelyGhost left a comment

Choose a reason for hiding this comment

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

We'll need to go through all lines of all of the files changed so we can confirm there weren't any (other) syntax errors created by reformatting the source code like this.

That said, I'm all for using black to center around some style guide. 👍

@TheLonelyGhost TheLonelyGhost self-assigned this Feb 24, 2019
@TheLonelyGhost TheLonelyGhost added this to Needs triage in First Aid Apr 10, 2019
@TheLonelyGhost
Copy link
Member

Closing because change is stale. Will need to address the issue and merge it quickly since it touches so many files and has a high potential for merge conflicts.

First Aid automation moved this from Needs triage to Closed Apr 10, 2019
@ghost ghost removed the in progress label Apr 10, 2019
@TheLonelyGhost TheLonelyGhost deleted the black branch June 17, 2019 01:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
First Aid
  
Closed
Development

Successfully merging this pull request may close these issues.

None yet

2 participants