Code review comment for lp:~jeff-licquia/pystromo/saitek

Revision history for this message
Raumkraut (raumkraut) wrote :

Right, further clarification:
- The events to use as additional LEDs should be passed in to InputDevices on instantiation. Actually, it looks like the value is already implicitly passed in with the **params. (The **params were originally intended solely to identify the devices to match against, so 'leds' may be better as another argument, I'm not sure)
- The LED state should probably only be saved on exit, which I think just means moving that code to the end of pystromo-remap.py.
- Nomenclature:
- - saitek.map should be named more specifically for the device.
- - The location of the save-file should probably not be user-specified, but in constants.py. Following XDG probably means ~/.local/pystromo/{device-id}.state (because we're storing the "state" of the device). If there's no device-id available (I forget), "{vendor}:{product}.state" probably covers 99.99% of cases.
- - Plus any other variables named after device manufacturers. ;)
- - And if the Czech wants to go all TRIGGER_HAPPY, I guess we should let him. :)

Also: `print` statements should probably be removed/commented from anything in the 'lib' directory, once you're happy that it's working.

Phew! It probably took me more time to write that, than it will for you to refactor the code. :D

review: Needs Fixing

« Back to merge proposal