Code review comment for lp:~henninge/launchpad/bug-506925-oops-export

Revision history for this message
Данило Шеган (danilo) wrote :

As discussed on IRC, I have some concerns with _try_encode_file_content method: it has very weird return semantics. Basically, they are: return encoded content according to translation_file header charset, and if it fails, return None if translation_file header wasn't UTF-8, otherwise fail with an exception. Not a very reasonable method, I'd say :)

We've looked at several solutions to that, one was to just switch the callsite to do exception handling, and not handle exceptions in this method (where the second one might fail, and would naturally propagate), though this approach loses some debugging information existing code has (file name in the exception error).

We can also just do this "exception error message improvement" in the _try_encode_file_content (i.e. catch the exception there and re-raise it unconditionally, compared to what we do now where we re-raise it only if the requested encoding was UTF-8).

review: Approve (code)

« Back to merge proposal