> 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