From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: Re: [edk2-devel] [PATCH v1 2/6] ArmVirtPkg: Add Platform CI and configuration for Core CI To: Ard Biesheuvel ,devel@edk2.groups.io From: "Sean" X-Originating-Location: Kirkland, Washington, US (50.35.74.15) X-Originating-Platform: Windows Chrome 83 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Thu, 09 Apr 2020 12:57:50 -0700 References: <3dcff902-cd2a-2189-245d-84548b2c7da0@arm.com> In-Reply-To: <3dcff902-cd2a-2189-245d-84548b2c7da0@arm.com> Message-ID: <12347.1586462270854537334@groups.io> Content-Type: multipart/alternative; boundary="Q1BTVDAvXvxn9dBoXrmA" --Q1BTVDAvXvxn9dBoXrmA Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Ard/Laszlo, On the topic of Package files. 1. The *.ci.yaml is designed to be at the root of the package and this fol= lows all other Core ci usage.=C2=A0 The Core CI will only look for this fil= e at the root as of now. 2.=C2=A0 The PlatformBuild.py file is exactly like a build.bat or build.sh= file found in many platform packages.=C2=A0 This file is relevant for loca= l dev as it will build the package and should support the feature set of th= e platform.=C2=A0 I would hope that the package maintainers would continue = to update this file to match the features of their package otherwise CI wil= l become less and less relevant overtime.=C2=A0 Thus I think it belongs in = the same place as build.sh.=C2=A0 The only other pattern we could follow is= a ".pytools" folder like for Core CI.=C2=A0 But again unless you strongly = disagree I would prefer the first. 3. The readme file...You tell me where?=C2=A0 Or it could be combined with= the existing readme.md file? 4. The iasl ext_dep file.=C2=A0 I could see this moving deeper into the pa= ckage.=C2=A0 Where would you like it? This file is an external dependency t= o control the version of iasl used on this platform.=C2=A0 It should be kep= t current with what the platform expects for iasl usage. Anyway, I need more actionable feedback if you don't agree with the above. On the forward slashes and typo.=C2=A0 I agree and will get them in the ne= xt version.=C2=A0 =C2=A0Thanks for the review. Thanks Sean --Q1BTVDAvXvxn9dBoXrmA Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable Ard/Laszlo,

On the topic of Package files. 
1. The *.c= i.yaml is designed to be at the root of the package and this follows all ot= her Core ci usage.  The Core CI will only look for this file at the ro= ot as of now. 
2.  The PlatformBuild.py file is exactly like= a build.bat or build.sh file found in many platform packages.  This f= ile is relevant for local dev as it will build the package and should suppo= rt the feature set of the platform.  I would hope that the package mai= ntainers would continue to update this file to match the features of their = package otherwise CI will become less and less relevant overtime.  Thu= s I think it belongs in the same place as build.sh.  The only other pa= ttern we could follow is a ".pytools" folder like for Core CI.  But ag= ain unless you strongly disagree I would prefer the first.  
3. The readme file...You tell me where?  Or it could be combined with= the existing readme.md file? 
4. The iasl ext_dep file.  I = could see this moving deeper into the package.  Where would you like i= t? This file is an external dependency to control the version of iasl used = on this platform.  It should be kept current with what the platform ex= pects for iasl usage. 

Anyway, I need more actionable feedb= ack if you don't agree with the above.  

On the forwar= d slashes and typo.  I agree and will get them in the next version.&nb= sp;  Thanks for the review. 

Thanks
Sean --Q1BTVDAvXvxn9dBoXrmA--