Skip to main content

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

  • From: Yiteng Zhang < >
  • To: ,
  • Subject: [pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant
  • Date: Mon, 12 Aug 2013 10:49:38 -0700

On 08/11/13 01:40 PM, Shawn Walker wrote:
On 08/09/13 18:54, Yiteng Zhang wrote:
On 08/ 9/13 02:32 PM, Shawn Walker wrote:
On 08/09/13 12:54, Yiteng Zhang wrote:
Hi guys,
I just fixed thsi bug and push my code review to the webrev. Please
take a look on it.
     http://ips.java.net/webrev/yitezhan/16986557_prefix_1/

     One thing to mention, after I run the test scripts, it reported
some mismatches failures. It is because the test scripts still expect to
see the output with prefixes. Thanks.


src/client.py:
  Unfortunately, this set of changes will drop the 'variant.' and
'facet.' prefixes from more than just the default output.  The
 original bug requests that the prefixes are only omitted from
 the default output.

 In other words, you should only omit the prefixes if the
 output_format != "default".

 Also, as a matter of style, we generally do not use '' to delimit
 strings or even single characters; we use "" instead.  (As seen
 on lines 4800, 4801, 4811, 4812, 4890, 4891.)


You should be able to fix all of the remaining tests by simply
changing __assert_facet_matches and __assert_variant_matches in
src/tests/cli/t_pkg_varcet.py to remove the 'facet.' or 'variant.'
prefix from the exp_def string variable.

-Shawn
Hi,
     Sorry about that I misunderstood the meaning of default output.
Thanks for your advise. Now I only omit the prefixed if the
output_format == "default" (I think you mean equal). Also, I modified

Yes, I meant output_format == "default"; sorry about that.

the t_pkg_varcet.py in order to pass the test cases. My code review can
be seen at:
     http://ips.java.net/webrev/yitezhan/16986557_prefix_2/

src/client.py:
  This is now correct.

src/tests/cli/t_pkg_varcet.py
  line 119: We always place a space before and after operators such as
     '-', '+', etc.  However, read on.

  lines 118-121:  This could be more simply written as:

    expected = "".join(
        (prefix + l)
        for l in expected.splitlines(True)
    )

After you make these changes, be certain you perform a full test suite run to ensure nothing else has broken.

You can run the test suite faster by using the -j option to have it run tests in parallel where possible, like this:

  python tests/run.py -j8

Assuming the test suite passes, you can push your changeset.

-Shawn
Hi Shawn,
Thanks, I have made these changes and a full test suite was passed. I wish I can push the code, but I am not authorized to write to the repository ips~pkg-gate.

-Yiteng


[pkg-discuss] Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Yiteng Zhang 08/09/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Danek Duvall 08/09/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Shawn Walker 08/09/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Yiteng Zhang 08/10/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Shawn Walker 08/11/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Yiteng Zhang 08/12/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Edward Pilatowicz 08/12/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Erik Trauschke 08/12/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Edward Pilatowicz 08/12/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Shawn Walker 08/12/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Edward Pilatowicz 08/12/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Shawn Walker 08/12/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Yiteng Zhang 08/12/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Edward Pilatowicz 08/15/2013

[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant

Yiteng Zhang 08/15/2013
 
 
Close
loading
Please Confirm
Close