> On Jan. 14, 2011, 1:16 p.m., Boroondas Gupte wrote:
> > scripts/install.py, lines 592-594
> > <http://codereview.secondlife.com/r/80/diff/1/?file=388#file388line592>
> >
> >     This can be written more pythonesque as
> >     
> >             (to_install, to_uninstall) = self._build_ifiles(platform, 
> > cache_dir)

Here, just
        to_install, to_uninstall = self._build_ifiles(platform, cache_dir)

which again constructs a tuple (now on the left hand side to receive the values 
of the returned tuple). Again, omitting the parentheses stresses that the 
returned tuple just serves as a temporary vehicle for the two values. This is 
also the prevalent style for unpacking multiple return values in the existing 
python code from the viewer codebase. (The style in functions for returning 
multiple values is less homogeneous. But the use of tuples for that purpose, 
with or without parentheses, seems established.)


> On Jan. 14, 2011, 1:16 p.m., Boroondas Gupte wrote:
> > scripts/install.py, line 549
> > <http://codereview.secondlife.com/r/80/diff/1/?file=388#file388line549>
> >
> >     I think for multiple return values, packaging in tuples is preferred 
> > over packaging in lists.[citation needed] I.e., use
> >     
> >             return (to_install, to_uninstall)

Actually, make that just
        return to_install, to_uninstall

The comma is enough for constructing the tuple, the parentheses aren't needed 
(in this context), and leaving them away IMHO makes more clear that we aren't 
really interested in the tuple as container, but just use it to return multiple 
values at once.


- Boroondas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/80/#review152
-----------------------------------------------------------


On Jan. 14, 2011, 12:26 p.m., Aleric Inglewood wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/80/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2011, 12:26 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> See https://jira.secondlife.com/browse/VWR-24311
> 
> Basically, this fixes the TODO comment in install.py but with the difference 
> that we really want to uninstall any old package with the same name, 
> different md5 or not.
> 
> 
> This addresses bug VWR-24311.
>     http://jira.secondlife.com/browse/VWR-24311
> 
> 
> Diffs
> -----
> 
>   doc/contributions.txt b0bd26c5638a 
>   scripts/install.py b0bd26c5638a 
> 
> Diff: http://codereview.secondlife.com/r/80/diff
> 
> 
> Testing
> -------
> 
> Loads of testing on linux... Installing new packages now cleanly removes the 
> old one first.
> 
> 
> Thanks,
> 
> Aleric
> 
>

_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Reply via email to