CR: Added BrewPlayer UI in clientapps

Here are my comments:

1) What directory in clientapps is this going in?

2) Wrong license header in brew.pcf - should
   be the tri-licensed RPSL/RCSL/GPL header.

3) No license header on BrewPlayer.c

4) The attachment for Umakefil's I see all
   have a .dat extension - I don't know why
   that is. It may be just something with an
   email client.

5) BrewWindow.c has no license header.

6) BrewWindow.h has no license header.

7) don't see a BIF entry for helix_restricted.bif

Rest looks good.

=============================================
Eric Hyche (ehychereal.com)
Technical Lead
RealNetworks, Inc.  

> -----Original Message-----
> From: clientapps-dev-bounceshelixcommunity.org 
> [mailto:clientapps-dev-bounceshelixcommunity.org] On
Behalf 
> Of Varun Kathuria
> Sent: Monday, March 10, 2008 9:01 AM
> To: clientapps-devhelixcommunity.org
> Subject: [Clientapps-dev] CR: Added BrewPlayer UI in
clientapps 
> 
> Synopsis:
>  
> Added BrewPlayer UI module in clientapps
> 
> 
> Overview:
> This module is created for the testing purpose of
brewext 
> which is already added in client as client/brewext.
>  
> Files Added:
> BrewWindow.c
> BrewPlayer.c
> brew.pcf
> BrewWindow.h
> CBREWPLAYER.bid
> umakefil
> brewplayerdll
> Elf2Mod
> brewplayer.mif
> brewplayer.brx
>  
> 
> Files Modified:
> 
>
buildbif-cvshelixclient-restrictedbuildBIFhxclient_3_1
_0
> _atlas_restricted.bif
> 
> Image Size and Heap Use impact (Client -Only):
> None.
> 
> Platforms and Profiles Affected:
> platform : brew-3.1.5-vc7-emulator (new SystemID)
> platform : brew-3.1.5-arm(new SystemID)
> profile    : helix-client-brewext (new profile)
> 
> Distribution Libraries Affected:
> None
> 
> Distribution library impact and planned action:
> None
> 
> Platforms and Profiles Build Verified:
> 
> BIF branch   -> hxclient_3_1_0_atlas_restricted
> Target(s)      -> brewplayer (new target)
> Profile          -> helix-client-brewext (new
profile)
> System ID    -> brew-3.1.5-vc7-emulator (new
SystemID)
> System ID    -> brew-3.1.5-arm(new SystemID)
> 
> Branch:
> HEAD,  hxclient_3_1_0_atlas
> 
> Files Attached:
> BrewWindow.c 
> BrewPlayer.c
> brew.pcf
> BrewWindow.h
> CBREWPLAYER.bid
> umakefil
> brewplayerdll
> Elf2Mod
> brewplayer.mif
> brewplayer.brx
> diffbif.txt
> 
> Thanks & Regards 
> Varun Kathuria
> 


_______________________________________________
Clientapps-dev mailing list
Clientapps-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/
clientapps-dev

Is there some reason why the clientapps/brewplayer target
is being put in helix_restricted.bif instead of helix.bif?
Seems like it should be in helix.bif.

In brewplayerdll:

> project.RemoveBuildOption('nodll')

Why do we need this?

>project.AddSources('%s' %
os.path.join(GetSDKPath('BREWSDK'), 'srcAEEAppGen.c'))

I don't think you need the string substitution, and it
doesn't make any sense to use os.path.join if you are
already going to provide a backslash in
"srcAEEAppGen.c".
Perhaps it should be:

project.AddSources(os.path.join(GetSDKPath("BREWSDK&quo
t;), "src", "AEEAppGen.c"))

and the same for the line below this.

In Elf2Mod: this file only contains this:

UmakefileVersion(2,1)

ProgramTarget('brewplayer')

I can't see how this could build anything since there's no
source or object files added. Are these added in a separate
.pcf file?


Rest looks good.

Eric

=============================================
Eric Hyche (ehychereal.com)
Technical Lead
RealNetworks, Inc.  

> -----Original Message-----
> From: Varun Kathuria [mailto:vkathuriareal.com]

> Sent: Thursday, March 13, 2008 3:47 AM
> To: ehychereal.com; clientapps-devhelixcommunity.org
> Subject: Re: [Clientapps-dev] CR: Added BrewPlayer UI
in clientapps 
> 
> Hi Eric,
>  
> - It will be a new directory "brewplayer" in
clientapps 
>  
> - Added license headers in brewplayer.c, brewwindow.c,

> brewwindow.h and changed license header of brew.pcf to

> tri-licensed RPSL/RCSL/GPL header
> 
> - Added brewplayer entry in helix_restricted.bif.
Please find 
> the attached diff . 
>  
> - Attached Umakefil's again.
> 
>  
> Thanks & Regards 
> Varun Kathuria
>  
> ----- Original Message ----- 
> From: "Eric Hyche" <ehychereal.com
<mailto:ehychereal.com> >
> To: "'Varun Kathuria'" <vkathuriareal.com

> <mailto:vkathuriareal.com> >; 
> <clientapps-devhelixcommunity.org 
> <mailto:clientapps-devhelixcommunity.org>
>
> Sent: Wednesday, March 12, 2008 9:04 PM
> Subject: RE: [Clientapps-dev] CR: Added BrewPlayer UI
in clientapps 
> 
> 
>  
> Here are my comments:
>  
> 1) What directory in clientapps is this going in?
> 
> 
>  2) Wrong license header in brew.pcf - should
>    be the tri-licensed RPSL/RCSL/GPL header.
>  
>  3) No license header on BrewPlayer.c
>  
> 4) The attachment for Umakefil's I see all
>    have a .dat extension - I don't know why
>    that is. It may be just something with an
>    email client.
> 
>  5) BrewWindow.c has no license header.
> 
>  6) BrewWindow.h has no license header.
>  
> 7) don't see a BIF entry for helix_restricted.bif
>  
> Rest looks good.
>  
> =============================================
> > Eric Hyche (ehychereal.com
<mailto:ehychereal.com> )
> > Technical Lead
> > RealNetworks, Inc.  
> > 
> >> -----Original Message-----
> >> From: clientapps-dev-bounceshelixcommunity.org 
> <mailto:clientapps-dev-bounceshelixcommunity.org>  
> >> [mailto:clientapps-dev-bounceshelixcommunity.org] On Behalf 
> >> Of Varun Kathuria
> >> Sent: Monday, March 10, 2008 9:01 AM
> >> To: clientapps-devhelixcommunity.org 
> <mailto:clientapps-devhelixcommunity.org> 
> >> Subject: [Clientapps-dev] CR: Added BrewPlayer
UI in clientapps 
> >> 
> >> Synopsis:
> >>  
> >> Added BrewPlayer UI module in clientapps
> >> 
> >> 
> >> Overview:
> >> This module is created for the testing purpose
of brewext 
> >> which is already added in client as
client/brewext.
> >>  
> >> Files Added:
> >> BrewWindow.c
> >> BrewPlayer.c
> >> brew.pcf
> >> BrewWindow.h
> >> CBREWPLAYER.bid
> >> umakefil
> >> brewplayerdll
> >> Elf2Mod
> >> brewplayer.mif
> >> brewplayer.brx
> >>  
> >> 
> >> Files Modified:
> >> 
> >>
buildbif-cvshelixclient-restrictedbuildBIFhxclient_3_1
_0
> >> _atlas_restricted.bif
> >> 
> >> Image Size and Heap Use impact (Client
-Only):
> >> None.
> >> 
> >> Platforms and Profiles Affected:
> >> platform : brew-3.1.5-vc7-emulator (new
SystemID)
> >> platform : brew-3.1.5-arm(new SystemID)
> >> profile    : helix-client-brewext (new
profile)
> >> 
> >> Distribution Libraries Affected:
> >> None
> >> 
> >> Distribution library impact and planned
action:
> >> None
> >> 
> >> Platforms and Profiles Build Verified:
> >> 
> >> BIF branch   ->
hxclient_3_1_0_atlas_restricted
> >> Target(s)      -> brewplayer (new target)
> >> Profile          -> helix-client-brewext
(new profile)
> >> System ID    -> brew-3.1.5-vc7-emulator
(new SystemID)
> >> System ID    -> brew-3.1.5-arm(new
SystemID)
> >> 
> >> Branch:
> >> HEAD,  hxclient_3_1_0_atlas
> >> 
> >> Files Attached:
> >> BrewWindow.c 
> >> BrewPlayer.c
> >> brew.pcf
> >> BrewWindow.h
> >> CBREWPLAYER.bid
> >> umakefil
> >> brewplayerdll
> >> Elf2Mod
> >> brewplayer.mif
> >> brewplayer.brx
> >> diffbif.txt
> >> 
> >> Thanks & Regards 
> >> Varun Kathuria
> >> 
> >
> 


_______________________________________________
Clientapps-dev mailing list
Clientapps-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/
clientapps-dev

Hi Eric,

Thanks for the suggestions.

1) We have added dependency of brewext in brewplayer.
Brewext is a "nodll" 
build.
    In that scenario we would need to remove
BuildOption('nodll') to make 
the player binary.

 2) Elf2Mod is required for arm build.
       Its just for converting elf format to MOD(brew binary
format).



Thanks & Regards
Varun Kathuria



----- Original Message ----- 
From: "Eric Hyche" <ehychereal.com>
To: "'Varun Kathuria'" <vkathuriareal.com>; 
<clientapps-devhelixcommunity.org>
Sent: Thursday, March 13, 2008 7:45 PM
Subject: RE: [Clientapps-dev] CR: Added BrewPlayer UI in
clientapps


>
> Is there some reason why the clientapps/brewplayer
target
> is being put in helix_restricted.bif instead of
helix.bif?
> Seems like it should be in helix.bif.
>
> In brewplayerdll:
>
>> project.RemoveBuildOption('nodll')
>
> Why do we need this?
>
>>project.AddSources('%s' %
os.path.join(GetSDKPath('BREWSDK'), 
>>'srcAEEAppGen.c'))
>
> I don't think you need the string substitution, and it
> doesn't make any sense to use os.path.join if you are
> already going to provide a backslash in
"srcAEEAppGen.c".
> Perhaps it should be:
>
>
project.AddSources(os.path.join(GetSDKPath("BREWSDK&quo
t;), "src", 
> "AEEAppGen.c"))
>
> and the same for the line below this.
>
> In Elf2Mod: this file only contains this:
>
> UmakefileVersion(2,1)
>
> ProgramTarget('brewplayer')
>
> I can't see how this could build anything since there's
no
> source or object files added. Are these added in a
separate .pcf file?
>
>
> Rest looks good.
>
> Eric
>
> =============================================
> Eric Hyche (ehychereal.com)
> Technical Lead
> RealNetworks, Inc.
>
>> -----Original Message-----
>> From: Varun Kathuria [mailto:vkathuriareal.com]
>> Sent: Thursday, March 13, 2008 3:47 AM
>> To: ehychereal.com; clientapps-devhelixcommunity.org
>> Subject: Re: [Clientapps-dev] CR: Added BrewPlayer
UI in clientapps
>>
>> Hi Eric,
>>
>> - It will be a new directory "brewplayer"
in clientapps
>>
>> - Added license headers in brewplayer.c,
brewwindow.c,
>> brewwindow.h and changed license header of brew.pcf
to
>> tri-licensed RPSL/RCSL/GPL header
>>
>> - Added brewplayer entry in helix_restricted.bif.
Please find
>> the attached diff .
>>
>> - Attached Umakefil's again.
>>
>>
>> Thanks & Regards
>> Varun Kathuria
>>
>> ----- Original Message ----- 
>> From: "Eric Hyche" <ehychereal.com
<mailto:ehychereal.com> >
>> To: "'Varun Kathuria'" <vkathuriareal.com
>> <mailto:vkathuriareal.com> >;
>> <clientapps-devhelixcommunity.org
>> <mailto:clientapps-devhelixcommunity.org>
>
>> Sent: Wednesday, March 12, 2008 9:04 PM
>> Subject: RE: [Clientapps-dev] CR: Added BrewPlayer
UI in clientapps
>>
>>
>>
>> Here are my comments:
>>
>> 1) What directory in clientapps is this going in?
>>
>>
>>  2) Wrong license header in brew.pcf - should
>>    be the tri-licensed RPSL/RCSL/GPL header.
>>
>>  3) No license header on BrewPlayer.c
>>
>> 4) The attachment for Umakefil's I see all
>>    have a .dat extension - I don't know why
>>    that is. It may be just something with an
>>    email client.
>>
>>  5) BrewWindow.c has no license header.
>>
>>  6) BrewWindow.h has no license header.
>>
>> 7) don't see a BIF entry for helix_restricted.bif
>>
>> Rest looks good.
>>
>> =============================================
>> > Eric Hyche (ehychereal.com
<mailto:ehychereal.com> )
>> > Technical Lead
>> > RealNetworks, Inc.
>> >
>> >> -----Original Message-----
>> >> From: clientapps-dev-bounceshelixcommunity.org
>> <mailto:clientapps-dev-bounceshelixcommunity.org>
>> >> [mailto:clientapps-dev-bounceshelixcommunity.org] On Behalf
>> >> Of Varun Kathuria
>> >> Sent: Monday, March 10, 2008 9:01 AM
>> >> To: clientapps-devhelixcommunity.org
>> <mailto:clientapps-devhelixcommunity.org>
>> >> Subject: [Clientapps-dev] CR: Added
BrewPlayer UI in clientapps
>> >>
>> >> Synopsis:
>> >>
>> >> Added BrewPlayer UI module in clientapps
>> >>
>> >>
>> >> Overview:
>> >> This module is created for the testing
purpose of brewext
>> >> which is already added in client as
client/brewext.
>> >>
>> >> Files Added:
>> >> BrewWindow.c
>> >> BrewPlayer.c
>> >> brew.pcf
>> >> BrewWindow.h
>> >> CBREWPLAYER.bid
>> >> umakefil
>> >> brewplayerdll
>> >> Elf2Mod
>> >> brewplayer.mif
>> >> brewplayer.brx
>> >>
>> >>
>> >> Files Modified:
>> >>
>> >>
buildbif-cvshelixclient-restrictedbuildBIFhxclient_3_1
_0
>> >> _atlas_restricted.bif
>> >>
>> >> Image Size and Heap Use impact (Client
-Only):
>> >> None.
>> >>
>> >> Platforms and Profiles Affected:
>> >> platform : brew-3.1.5-vc7-emulator (new
SystemID)
>> >> platform : brew-3.1.5-arm(new SystemID)
>> >> profile    : helix-client-brewext (new
profile)
>> >>
>> >> Distribution Libraries Affected:
>> >> None
>> >>
>> >> Distribution library impact and planned
action:
>> >> None
>> >>
>> >> Platforms and Profiles Build Verified:
>> >>
>> >> BIF branch   ->
hxclient_3_1_0_atlas_restricted
>> >> Target(s)      -> brewplayer (new
target)
>> >> Profile          ->
helix-client-brewext (new profile)
>> >> System ID    -> brew-3.1.5-vc7-emulator
(new SystemID)
>> >> System ID    -> brew-3.1.5-arm(new
SystemID)
>> >>
>> >> Branch:
>> >> HEAD,  hxclient_3_1_0_atlas
>> >>
>> >> Files Attached:
>> >> BrewWindow.c
>> >> BrewPlayer.c
>> >> brew.pcf
>> >> BrewWindow.h
>> >> CBREWPLAYER.bid
>> >> umakefil
>> >> brewplayerdll
>> >> Elf2Mod
>> >> brewplayer.mif
>> >> brewplayer.brx
>> >> diffbif.txt
>> >>
>> >> Thanks & Regards
>> >> Varun Kathuria
>> >>
>> >
>>
> 


_______________________________________________
Clientapps-dev mailing list
Clientapps-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/
clientapps-dev