On 10/21/13 10:58 AM, Yiteng Zhang wrote:
On 10/ 2/13 12:02 PM, Yiteng Zhang wrote:
On 08/15/13 04:35 PM, Yiteng Zhang wrote:
On 08/15/13 04:27 PM, Edward Pilatowicz wrote:
On Mon, Aug 12, 2013 at 02:20:59PM -0700, Shawn Walker wrote:Hi Edward,
On 08/12/13 10:49, Yiteng Zhang wrote:hey Yiteng,
On 08/11/13 01:40 PM, Shawn Walker wrote:As soon as you've had a few successfully reviewed changesets,
On 08/09/13 18:54, Yiteng Zhang wrote:Hi Shawn,
On 08/ 9/13 02:32 PM, Shawn Walker wrote:Yes, I meant output_format == "default"; sorry about that.
On 08/09/13 12:54, Yiteng Zhang wrote:Hi,
I just fixed thsi bug and push my code review to the webrev.
take a look on it.
One thing to mention, after I run the test scripts, it
some mismatches failures. It is because the test scripts still
see the output with prefixes. Thanks.
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.
Sorry about that I misunderstood the meaning of default
Thanks for your advise. Now I only omit the prefixed if the
output_format == "default" (I think you mean equal). Also, I
the t_pkg_varcet.py in order to pass the test cases. My codesrc/client.py:
be seen at:
This is now correct.
line 119: We always place a space before and after operators
'-', '+', 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
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.
Thanks, I have made these changes and a full test suite was
I wish I can push the code, but I am not authorized to write to the
someone will nominate you for commit access and you'll be able to
putback. Until then, another pkg team member can push your changes.
In this particular case, Ed is right, there's unfortunately some
logic in the kernel zone installation scripts that currently relies
on the 'facet.' prefixes that Ed and I both missed during our
previous review of the zone scripts.
Until 17301112 is fixed, we won't be able to push these changes.
So for now, just hold onto your workspace and sometime during ON
build 30, we should be able to push them.
fyi, i just pushed 17301112 into ON gate for build s12_30. so once the
pkg gate closes for s12_29 (and opens for s12_30) you should be OK to
push this fix.
I appreciate it. I will check the bug again when s12_30 opens. Thanks.
I think the pkg gate is open for s12_30 now. I have tested this fix
and would like to push it. Thanks.
Can everyone take a look on it and help me to push this fix?
Where you ever able to actually install a kernel zone successfully? Even if there is a very minor risk of this breaking zones you should do at least a manual test.
I know you had some issues testing zones for a different bug so I was curious if you ever hashed that out. It seems like a good idea to just do a quick zone test with your chances before you put back.
[pkg-discuss] Re: Code review request for Bug 16986557 - prefixes should be omitted from output of facet and variant