bbfrederick/rapidtide

Run black automated code formatter on package

Closed this issue · 9 comments

tsalo commented

This stems from #46 (comment). Prior to releasing 2.0.0, we should run black on the full package to standard the formatting of the code. This will have the added benefit of minimizing future diffs to better isolate relevant changes.

tsalo commented

@bbfrederick are you planning to deal with the merge conflicts between dev and master when you release 2.0.0, or will you just have dev overwrite master? I ask because running black on dev would introduce a massive number of merge conflicts if you're planning to merge the two branches manually.

I was planning to simply overwrite master with dev. Is there a reason that would be a bad thing?

tsalo commented

It could be a problem if you push changes to master that you ultimately wanted supported in dev, but then didn't reimplement on dev. Other than that, I don't think there should be any issues.

tsalo commented

In that case, it seems like it should be okay to start running black at any time. Given the sheer size of rapidtide, it might be good to do it in a series of PRs so that the changes are more digestible (unless you want to run it all at once yourself). Would it be cool for me to start submitting style PRs? If so, are there any modules I should avoid touching at the moment?

I've only backported critical fixes I've made to dev to master for several months, so I don't think that will be a problem.

Re: starting to do style PR's, go for it. The files I'm working on most actively at the moment are workflows/rapidtide.py, workflows/rapidtide_parser.py, workflows/rapidtide2x_parser.py, workflows/happy.py, refine.py and stats.py. Anything else you can probably start doing in chunks and I'd never notice. For example, the entire scripts directory.

tsalo commented

I ran into some technical issues a few days ago but I'll hopefully be able to start opening PRs by the end of the week.

tsalo commented

After a bit more thought, I'm not sure if I should run black. It would artificially make it seem like I rewrote the whole library in the commit history. Would you be willing to run black and push the changes yourself instead? You can pip install black and then to run it you just do black /path/to/rapidtide/ from the command line.

tsalo commented

Closed by 042d2cf.

Just so you know, I've enabled the Black integration into PyCharm, so this should just happen automatically going forward.