Merge lp://qastaging/~ralsina/ubuntuone-control-panel/opt-parsing into lp://qastaging/ubuntuone-control-panel

Proposed by Roberto Alsina
Status: Merged
Approved by: Roberto Alsina
Approved revision: 267
Merged at revision: 267
Proposed branch: lp://qastaging/~ralsina/ubuntuone-control-panel/opt-parsing
Merge into: lp://qastaging/ubuntuone-control-panel
Diff against target: 269 lines (+161/-36)
6 files modified
bin/ubuntuone-control-panel-qt (+1/-30)
run-tests (+1/-1)
ubuntuone/controlpanel/gui/qt/main/__init__.py (+36/-4)
ubuntuone/controlpanel/gui/qt/main/tests/__init__.py (+17/-0)
ubuntuone/controlpanel/gui/qt/main/tests/test_main.py (+105/-0)
ubuntuone/controlpanel/gui/qt/uniqueapp/linux.py (+1/-1)
To merge this branch: bzr merge lp://qastaging/~ralsina/ubuntuone-control-panel/opt-parsing
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
dobey (community) Approve
Diego Sarmentero (community) Approve
Review via email: mp+94599@code.qastaging.launchpad.net

Commit message

* Cleanup the ubuntuone-control-panel-qt script moving all logic into the main module.
* Adds tests for main()
* Parse Qt options correctly
* Move some code from UniqueApp to main where it belongs, and add tests for it
* Migrate to argparse

Description of the change

* Cleanup the ubuntuone-control-panel-qt script moving all logic into the main module.
* Adds tests for main()
* Parse Qt options correctly
* Move some code from UniqueApp to main where it belongs, and add tests for it
* Migrate to argparse

To post a comment you must log in.
Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

+1

review: Approve
Revision history for this message
dobey (dobey) :
review: Approve
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Any chance you migrate to argparse while you're at this branch? :-)

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Thanks for this work!

Could you please re-align all the options in parse_args? The multi-line statements are misaligned, for example, lines 80 to 84 should be:

80 + result.add_argument("--switch-to", dest="switch_to",
81 + metavar="PANEL_NAME", default="",
82 + help="Start Ubuntu One in the "
83 + "PANEL_NAME tab. Possible values are: "
84 + "dashboard, volumes, devices, applications")

or (the one I prefer):

80 + result.add_argument("--switch-to", dest="switch_to",
81 + metavar="PANEL_NAME", default="",
82 + help="Start Ubuntu One in the PANEL_NAME tab. "
83 + "Possible values are: dashboard, volumes, devices, applications")

(the chosen pattern should be applied to the rest of the arguments).

review: Needs Fixing
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

(the examples above can be properly read -regarding spacing- in the generated email with the review)

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Looks good!

review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :

Attempt to merge into lp:ubuntuone-control-panel failed due to conflicts:

text conflict in bin/ubuntuone-control-panel-qt

267. By Roberto Alsina

merged trunk, resolved conflict

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches