Checkbox

Merge lp://staging/~taihsiangho/checkbox/fix-1334224 into lp://staging/checkbox

fix-1334224
Merge into trunk

Proposed by Taihsiang Ho on 2014-08-13

Status:

Rejected

Rejected by:

Zygmunt Krynicki on 2014-08-18

Proposed branch:

lp://staging/~taihsiangho/checkbox/fix-1334224

Merge into:

lp://staging/checkbox

Diff against target:

52 lines (+22/-12)

1 file modified

checkbox-support/checkbox_support/parsers/udevadm.py (+22/-12)

To merge this branch:

bzr merge lp://staging/~taihsiangho/checkbox/fix-1334224

High

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Zygmunt Krynicki (community)	2014-08-13	Needs Fixing on 2014-08-13
Daniel Manrique	2014-08-13	Pending
Review via email: mp+230600@code.staging.launchpad.net

This proposal supersedes a proposal from 2014-08-06.

Description of the change

I found another card reader with the same issue so
I add the card reader into the workaround.

Rearrange the code and make it easier to read.

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2014-08-06: Posted in a previous version of this proposal

review: Approve

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2014-08-13:

FYI: I'd like to see two patches:

- rework the code to make it easier to read
- add new features

In addition to that, please use under_score naming theme for methods/functions.
Iff you want to apply those changes I can show you how to do them in git in a few minutes (hangout?) otherwise we can land it as is but I'll be just a bit more grumpy each time it happens.

For methods, please keep comments _inside_ the method, better yet, use a docstring and document what the method does, what it needs and what it produces.

So again, I'm setting this to needs fixing but if you wish we can land it as-is

review: Needs Fixing

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2014-08-18:

This now lands elsewhere: https://code.launchpad.net/~zkrynicki/checkbox/rework/+merge/230613