D2244: histedit: binascii.unhexlify (aka node.bin) throws new exception type on py3

2018-02-13 Thread martinvonz (Martin von Zweigbergk)
martinvonz added a comment. In https://phab.mercurial-scm.org/D2244#37261, @indygreg wrote: > Now I'm second guessing accepting/committing this... Some thoughts: - This is probably not very well tested code (because it's an error path) - As long as we grep for TypeError and

D2244: histedit: binascii.unhexlify (aka node.bin) throws new exception type on py3

2018-02-13 Thread indygreg (Gregory Szorc)
indygreg added a comment. Now I'm second guessing accepting/committing this... REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2244 To: durin42, #hg-reviewers, indygreg Cc: martinvonz, indygreg, mercurial-devel ___ Merc

D2244: histedit: binascii.unhexlify (aka node.bin) throws new exception type on py3

2018-02-13 Thread martinvonz (Martin von Zweigbergk)
martinvonz added a comment. In https://phab.mercurial-scm.org/D2244#37169, @durin42 wrote: > I could go either way. A casual grep for "except TypeError" suggests we're looking at 4 or 5 locations *total*, so my bias is to do it at the callsites and avoid the annoyingly-large overhead of

D2244: histedit: binascii.unhexlify (aka node.bin) throws new exception type on py3

2018-02-13 Thread durin42 (Augie Fackler)
This revision was automatically updated to reflect the committed changes. Closed by commit rHG88dd15b38bb0: histedit: binascii.unhexlify (aka node.bin) throws new exception type on py3 (authored by durin42, committed by ). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercu

D2244: histedit: binascii.unhexlify (aka node.bin) throws new exception type on py3

2018-02-13 Thread indygreg (Gregory Szorc)
indygreg accepted this revision. indygreg added a comment. This revision is now accepted and ready to land. In https://phab.mercurial-scm.org/D2244#37169, @durin42 wrote: > I could go either way. A casual grep for "except TypeError" suggests we're looking at 4 or 5 locations *total*, so m

D2244: histedit: binascii.unhexlify (aka node.bin) throws new exception type on py3

2018-02-13 Thread durin42 (Augie Fackler)
durin42 added a comment. I could go either way. A casual grep for "except TypeError" suggests we're looking at 4 or 5 locations *total*, so my bias is to do it at the callsites and avoid the annoyingly-large overhead of a wrapper function for something that's so fundamental. REPOSITORY rH

D2244: histedit: binascii.unhexlify (aka node.bin) throws new exception type on py3

2018-02-13 Thread indygreg (Gregory Szorc)
indygreg requested changes to this revision. indygreg added a comment. This revision now requires changes to proceed. I'm not super thrilled about this. Presumably we'll have this class of failure throughout the code base. What do you think about having `node.bin()` catch `binascii.Error` and

D2244: histedit: binascii.unhexlify (aka node.bin) throws new exception type on py3

2018-02-13 Thread durin42 (Augie Fackler)
durin42 created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY Lucky for us, the exception type exists on 2.7, so we can include it in the except block without any extra work. REPOSITORY rHG Mercurial REVISION DETAIL htt