Page MenuHomePhabricator

lfs: use str for the open() mode when opening a blob for py3
ClosedPublic

Authored by mharbison72 on Feb 8 2020, 11:48 PM.

Details

Summary

The other fix for this was to leave the mode as bytes, and import
pycompat.open() like a bunch of other modules do. But I think it's confusing
to still use bytes at the python boundary, and obviously error prone. Grepping
for open\(.+, ['"][a-z]+['"]\) and open\(.+, b['"][a-z]+['"]\) outside of
tests, there are 51 and 87 uses respectively, so it's not like this is a rare
direct usage.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mharbison72 created this revision.Feb 8 2020, 11:48 PM
pulkit accepted this revision.Feb 10 2020, 11:30 AM
This revision is now accepted and ready to land.Feb 10 2020, 11:30 AM
pulkit requested changes to this revision.Feb 10 2020, 1:10 PM

This one fails to apply on tip of stable branch. Can you rebase and resend?

This revision now requires changes to proceed.Feb 10 2020, 1:10 PM
mharbison72 requested review of this revision.Feb 10 2020, 1:57 PM

This one fails to apply on tip of stable branch. Can you rebase and resend?

I think the thing this is fixing is on default

durin42 accepted this revision.Feb 10 2020, 4:39 PM
durin42 added a subscriber: durin42.

Looks fine to me for default, queued.

Forgot to say: I'd be a big fan of unwinding pycompat.open() now that we're done with the module transformer.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.