Code review comment for lp://qastaging/~ted/snapcraft/ros-catkin-plugin

Revision history for this message
Leo Arias (elopio) wrote :

It's probably not a good idea to use config={} in the arguments of the methods because {} is mutable and it can cause some unexpected behaviors. See more info here: http://effbot.org/zone/default-values.htm
As the article suggest, it's better to use config=None, and then if config is None: config = {}.

Also, your python files are not passing the mccabe test:

./snapcraft/plugins/catkin.py:73:1: C901 'CatkinPlugin.find_package_deps' is too complex (10)
./snapcraft/plugins/catkin.py:128:1: C901 'CatkinPlugin.build' is too complex (13)

You should extract methods for the loops. The more the merrier.

« Back to merge proposal