Skip to main content

[pkg-discuss] Re: review request for 17173472 - pkg change-variant should allow variant removal

  • From: Edward Pilatowicz < >
  • To:
  • Subject: [pkg-discuss] Re: review request for 17173472 - pkg change-variant should allow variant removal
  • Date: Fri, 4 Oct 2013 14:26:39 -0700

On Fri, Oct 04, 2013 at 01:08:51PM -0700, Xiaobo Shen wrote:
> On 10/ 3/13 03:47 PM, Danek Duvall wrote:
> >Xiaobo Shen wrote:
> >
> >>Alta helped modifying the man page. here is a new webrev. Please take a
> >>look. Thanks.
> >>https://ips.java.net/webrev/xiaoshen/Bug_17173472_pkgvariant3/
> >Your webrev is against a very old version of the gate, which doesn't
> >include all the changes I synced in recently.  Please pull, merge, and post
> >a new webrev.
> >
> >Thanks,
> >Danek
> Ok. Here is an updated one. Thanks.
> https://ips.java.net/webrev/xiaoshen/Bug_17173472_pkgvariant5/
>

out of curiosity i took a quick look at this.

- in image_config_update() you look in new_variants for variants with a
  value of "NONE".  i may be mistaken, but i don't think you'll ever
  find any because in make_change_varcets_plan() you removed them all.
  (the "variants" variable in make_change_varcets_plan() becomes the
  new_variants variable you see later in image_config_update().

- you've changed the behavior of make_change_varcets_plan() such that it
  could have side effects.  previously, it never changed the "variants"
  dictionary that was passed in (it created a new dictionary with
  different contents).  with your changes you're now modifying the
  dictionary that was passed in by the caller.

ed


[pkg-discuss] Re: review request for 17173472 - pkg change-variant should allow variant removal

Xiaobo Shen 10/03/2013

[pkg-discuss] Re: review request for 17173472 - pkg change-variant should allow variant removal

Danek Duvall 10/03/2013

[pkg-discuss] Re: review request for 17173472 - pkg change-variant should allow variant removal

Xiaobo Shen 10/04/2013

[pkg-discuss] Re: review request for 17173472 - pkg change-variant should allow variant removal

Edward Pilatowicz 10/04/2013

[pkg-discuss] Re: review request for 17173472 - pkg change-variant should allow variant removal

Xiaobo Shen 10/04/2013

[pkg-discuss] Re: review request for 17173472 - pkg change-variant should allow variant removal

Edward Pilatowicz 10/04/2013

[pkg-discuss] Re: review request for 17173472 - pkg change-variant should allow variant removal

Xiaobo Shen 10/21/2013
 
 
Close
loading
Please Confirm
Close