Code review comment for ~rafaeldtinoco/ubuntu/+source/qemu:lp1805256-bionic-refix

Revision history for this message
dann frazier (dannf) wrote :

@rafaeldtinoco: Testing all looks good on arm64/armhf. Can you change the name of the patch in debian/changelog to match the new name of the patch? It is missing the -arm-only suffix. With that, I'm +1. But, if you haven't already, I'd recommend testing w/ unifdef just to prove that the !aarch64 code is exactly what it was before.

btw, though I understand the reasoning, I'm personally not a fan of the #ifdef approach as that may cause maintenance headaches in upcoming years. If the submitter of bug 1885419 were to be kind enough to verify a non-ifdef'd version in his environment, would that give you enough confidence to ship it? Or is the concern about not-yet-uncovered regressions?

Thanks for continuing to stick with this nasty issue for so long Rafael!

 -dann

« Back to merge proposal