Re: [PATCH v2 edk2-platforms 16/20] Platform/Broadcom/RPi3: Add Raspberry Pi 3 Platform

[Pete Batard <pete@akeo.ie>]

Hi Ard,

Thanks for the review. I will incorporate the points you raised for 8/9/13.

On 2018.12.14 15:39, Ard Biesheuvel wrote:
> On Mon, 10 Dec 2018 at 13:39, Pete Batard <pete@akeo.ie> wrote:
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
>>   Platform/Broadcom/Bcm283x/RaspberryPiPkg.dec |  63 ++
>>   Platform/Broadcom/Bcm283x/RaspberryPiPkg.dsc | 636 ++++++++++++++++++++
>>   Platform/Broadcom/Bcm283x/RaspberryPiPkg.fdf | 450 ++++++++++++++
>>   Platform/Broadcom/Bcm283x/Readme.md          | 263 ++++++++
>>   4 files changed, 1412 insertions(+)
>>
> 
> This looks fine as well.
> 
> In general, please screen the code for the cosmetic stuff I mentioned.
> I haven't looked at the SD/MMC drivers yet, and I don't have the
> bandwidth (or the energy) to do a deep review of those. The NV storage
> driver I will have a more careful look at, but not today - hopefully
> by the end of next week,
> 
> 
> I tried running PatchCheck.py but it doesn't seem to run on my system
> (I have never used it in anger myself), but it should help you spot
> many of the issues I brought up.

I ran PatchCheck.py on every single patch sent, and made sure the script 
didn't report any issue.

For instance, this is what PatchCheck reports on my Debian system for 
the 16 patches that apply to edk2-platforms:

---------------------------------------------------------------------

# python /usr/src/edk2/BaseTools/Scripts/PatchCheck.py -16
Checking git commit: 4695ab7f1bc7
The commit message format passed all checks.
The code passed all checks.

Checking git commit: 4583a4c82bba
The commit message format passed all checks.
The code passed all checks.

Checking git commit: 7406c575c809
The commit message format passed all checks.
The code passed all checks.

Checking git commit: b5ee3fe94aae
The commit message format passed all checks.
The code passed all checks.

Checking git commit: 937845c4cc4d
The commit message format passed all checks.
The code passed all checks.

Checking git commit: 07dae6403e5e
The commit message format passed all checks.
The code passed all checks.

Checking git commit: cef8f008d1fa
The commit message format passed all checks.
The code passed all checks.

Checking git commit: 5304797f8494
The commit message format passed all checks.
The code passed all checks.

Checking git commit: 17711dc8965d
The commit message format passed all checks.
The code passed all checks.

Checking git commit: 83627cdc550f
The commit message format passed all checks.
The code passed all checks.

Checking git commit: c4f87ea8987b
The commit message format passed all checks.
The code passed all checks.

Checking git commit: ca99e77c7b95
The commit message format passed all checks.
The code passed all checks.

Checking git commit: eddd965e2672
The commit message format passed all checks.
The code passed all checks.

Checking git commit: 0893ac6d1509
The commit message format passed all checks.
The code passed all checks.

Checking git commit: f595d0be1e86
The commit message format passed all checks.
The code passed all checks.

Checking git commit: fe4b945ac743
The commit message format passed all checks.
The code passed all checks.

---------------------------------------------------------------------

So, it doesn't look like the script is current set to detect space 
before parenthesis and the other cosmetic issues you pointed out. But 
I'll make sure to get that fixed. I am currently looking into having the 
Visual Studio editor automate the process of fixing this.

Note that I also found that 'git am' complained about empty newlines 
being present at the end of some files, so I'll fix that too.

Regards,

/Pete