undefined | Better HN

0 pointsbombolo3y ago0 comments

> This is a bad practice. You should be using black as a pre-commit hook

I would reject such commits in review.

A human might add one or two items to a list and black might decide it's now too long, and make 1 line into 10 lines.

Now I have to manually compare the list item by item to figure out what has changed.

So I normally require formatting to be done in a separate commit, because I don't want to review the larger than necessary diffs that come out doing it within the same commit.

0 comments

eesmith3y ago

> A human might add one or two items to a list and black might decide it's now too long, and make 1 line into 10 lines.

A human might add one or two items to a list, decide it's now too long, and make 1 line into 10 lines.

Including the same hypothetical first contributor you mentioned earlier, who you think will find using requirements.txt as being too big a barrier to entry.

Onboarding occurs either way.

I get that you don't like using black - and that's fine! I don't use black on my project either.

But it seems like you're trying to find some other reason to reject black, and constructing hypotheticals that don't make any sense.

Just say you don't like black's choices, and leave it at that.

bomboloOP3y ago

> A human might add one or two items to a list, decide it's now too long, and make 1 line into 10 lines.

At which point I tell him to split formatting and actual changes into different commits (see https://mtlynch.io/code-review-love/).

> I get that you don't like using black - and that's fine! I don't use black on my project either.

Well according to this comment, it's because we are noobs: "the people that disagree just haven't figured out that they're wrong yet"

> But it seems like you're trying to find some other reason to reject black, and constructing hypotheticals that don't make any sense.

After the n-th time I have to re-setup the entire virtual env on a different branch just to re-run black and isort to backport a fix to a release branch… it does get tiring.

I presume most people here just do websites and don't really have a product that is released to customers who pay to support old versions for years, so black changing syntax is a 1 time event rather than a continuous source of daily pain.

But it seems the commentators here don't have the experience to know there might be a use case they didn't think of.

eesmith3y ago

> and don't really have a product that is released to customers who pay to support old versions for years

My main product is 12 years old, with paying support customers, and with bugfix branches for older releases.

> just to re-run black and isort to backport a fix to a release branch

Great! That's an excellent reason. But it has nothing to with bisection.

j / k navigate · click thread line to collapse

0 comments

eesmith3y ago

> A human might add one or two items to a list and black might decide it's now too long, and make 1 line into 10 lines.

A human might add one or two items to a list, decide it's now too long, and make 1 line into 10 lines.

Including the same hypothetical first contributor you mentioned earlier, who you think will find using requirements.txt as being too big a barrier to entry.

Onboarding occurs either way.

I get that you don't like using black - and that's fine! I don't use black on my project either.

But it seems like you're trying to find some other reason to reject black, and constructing hypotheticals that don't make any sense.

Just say you don't like black's choices, and leave it at that.

bomboloOP3y ago

> A human might add one or two items to a list, decide it's now too long, and make 1 line into 10 lines.

At which point I tell him to split formatting and actual changes into different commits (see https://mtlynch.io/code-review-love/).

> I get that you don't like using black - and that's fine! I don't use black on my project either.

Well according to this comment, it's because we are noobs: "the people that disagree just haven't figured out that they're wrong yet"

> But it seems like you're trying to find some other reason to reject black, and constructing hypotheticals that don't make any sense.

After the n-th time I have to re-setup the entire virtual env on a different branch just to re-run black and isort to backport a fix to a release branch… it does get tiring.

But it seems the commentators here don't have the experience to know there might be a use case they didn't think of.

eesmith3y ago

> and don't really have a product that is released to customers who pay to support old versions for years

My main product is 12 years old, with paying support customers, and with bugfix branches for older releases.

> just to re-run black and isort to backport a fix to a release branch

Great! That's an excellent reason. But it has nothing to with bisection.

j / k navigate · click thread line to collapse