Code review comment for lp:~stefan-schwarzburg/qreator/touch_qrcodesize

David Planella (dpm) wrote :

Thanks Stefan, indeed, time to start the fun with the qml version ;)

The change looks good to me. However, may I ask you to fix just one thing? The modification includes 2 changes:

1) Addition of the new lookup tables from 11 to 40
2) Whitespace change from tabs to spaces under RS_BLOCK_TABLE

The reason I'm asking, apart from nitpicking (sorry) is because this way the change is much more easy to read. I assume you haven't changed the arrays from 1 to 10, but that's not obvious from the diff, as it's quite a long list of values to check one by one. Also, although I prefer spaces than tabs myself, for consistency's sake we should stick to the whitespace used originally rather than mixing spaces and tabs.

It will also help if upstream wants to cherrypick this one change but not more intrusive ones.

Thanks!

review: Needs Fixing

« Back to merge proposal