Merge lp:~stefan-schwarzburg/qreator/touch_qrcodesize into lp:qreator/touch
Status: | Merged |
---|---|
Approved by: | David Planella |
Approved revision: | 16 |
Merged at revision: | 14 |
Proposed branch: | lp:~stefan-schwarzburg/qreator/touch_qrcodesize |
Merge into: | lp:qreator/touch |
Diff against target: |
201 lines (+182/-2) 1 file modified
js/qrcode.js (+182/-2) |
To merge this branch: | bzr merge lp:~stefan-schwarzburg/qreator/touch_qrcodesize |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
David Planella | Approve | ||
Review via email: mp+164960@code.launchpad.net |
Description of the change
Hi David,
Since you did so much great work on releasing the python based version, we are now at a point were we can continue with the qml version, I guess.
So: this is the first change I would like to input.
It is an hopefully uncontroversionl change, it extends the possible qrcode size of the javascript library to version 40 (previously the max size was version 10). The only reason why we might not want this here is that it could also go into the upstream version. However, there will be other and deeper changes to the qrcode.js library in the future, which will simplify calculating the best size before creating the code... So I think qreator is the right place for this patch.
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!