Tao Klerks <tao@xxxxxxxxxx> writes: > > > On Fri, Jul 8, 2022 at 10:01 AM Kilian Kilger via GitGitGadget > <gitgitgadget@xxxxxxxxx> wrote: >> >> From: Kilian Kilger <kilian.kilger@xxxxxxx> >> >> The Perforce client name can contain arbitrary characters >> which do not decode to UTF-8. Use the fallback strategy >> implemented in metadata_stream_to_writable_bytes() also >> for the client name. >> >> Signed-off-by: Kilian Kilger <kkilger@xxxxxxxxx> >> --- >> ... >> >> @@ -871,6 +871,8 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False, >> continue >> if 'desc' in entry: >> entry['desc'] = metadata_stream_to_writable_bytes(entry['desc']) >> + if 'client' in entry: >> + entry['client'] = metadata_stream_to_writable_bytes(entry['client']) >> if 'FullName' in entry: >> entry['FullName'] = metadata_stream_to_writable_bytes(entry['FullName'] We had two repetitions and now we have three, which is a good time to see if it makes sense to reduce the temptation for future developers to add the fourth repetition in the next round, e.g. for e in ["client", "desc", "FullName"]: if e in entry: entry[e] = metadata_stream_to_writable_bytes(entry[e]) or something like that? > This makes sense to me, and I don't see anything wrong with the "form" > (and nor does GitGitGadget). One thing that is a bit problematic is that in-body From does not match the sign-off. Kilian, which identity do you want to use in your contribution to this project? > Not sure whether formal review sign-off is used on this list, I don't > tend to see it, but do I see "Reviewed-by" on patches, so FWIW: > > Reviewed-by: Tao Klerks <tao@xxxxxxxxxx> Thanks.