Rich,

Thanks for the review. Comments inline.

On 09/25/12 01:54 AM, Rich Burridge wrote:
On 09/24/2012 10:25 AM, Srinivasa Sarva wrote:

Please review the changes for
7200300 Upgrade sox to 14.4.0

Patches 1 to 7 are header changes only.
Patch 8 is new.

Webrev
http://jurassic.us.oracle.com/net/ssarva-us/export/home/srini/S11U2/WS4/webrev/


Why change all the patches to level 0? It doesn't gain you anything.
It's perfectly fine if the first part of the pathname doesn't match the
current version. Lots of components have these sort of patches.

    I didn't want to leave old version number in the patches when we
can use patch level 0. Just a one time cleanup step, but I'm ok either way.



I would suggest not putting the caseid all on one line in the .p5m file.
Leave it the way it was. It's a gratuitous change that's not needed.

    Done.


If you just changed the things that really needed fixing your webrev would
be much smaller and it would be much easier to see what had really
changed and get a review.

    Webrev
http://jurassic.us.oracle.com/net/ssarva-us/export/home/srini/S11U2/WS4/webrev/


Did you build it on x86 and SPARC? Can you point me at your build logs?

    x86:
/net/ssarva-us/export/home/srini/S11U2/WS4-1/components/sox/build_log_x86

    Sparc:
/net/ssarva-us/export/home/srini/S11U2/WS4-1/components/sox/build_log_sparc




How did you test it? Can you point me at your results?
I see the NO_TESTS in the Makefile, but surely you testing
this in some way ...

    Yes, there are no tests available from community.
I tested using 2 .wav files. Tests like checking the duration of .wav file, samples, mix two .wav files, play a wav file etc. just the basic stuff to make sure its not broken.

    Test log:
     /net/ssarva-us/export/home/srini/S11U2/WS4-1/components/sox/testlog


The Bugster CR is still in a Dispatched state. Are you going to add an
Evaluation, Suggested Fix and adjust it to "Fix In progress"? Adding how
you tested it (and the test results) to the bug would also be good.

I moved it to "Accepted" now. I usually fill evaluation, suggested fix and also attach a doc with all the code changes after the webrev is approved and before the RTI.


    Thanks
    Srini



_______________________________________________
userland-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/userland-discuss

Reply via email to