Re: [edk2-devel] [PATCH 3/3] OvmfwPkg: Don't exclude XCODE Modules

["Andrew Fish" <afish@apple.com>]

> On May 26, 2020, at 4:45 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 05/26/20 06:10, Andrew Fish wrote:
>> 
>> 
>>> On May 25, 2020, at 12:31 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>>> 
>>> Hi Andrew,
>>> 
>>> On 05/24/20 23:20, Andrew Fish via groups.io <http://groups.io/> wrote:
>>>> With this BZ getting fixed we no longer need to special case XCODE.
>>>> 
>>>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=557
>>>> Signed-off-by: Andrew Fish <afish@apple.com>
>>>> 
>>>> Signed-off-by: Andrew Fish <afish@apple.com>
>>>> ---
>>>> OvmfPkg/OvmfPkgIa32.dsc    | 3 +--
>>>> OvmfPkg/OvmfPkgIa32.fdf    | 2 --
>>>> OvmfPkg/OvmfPkgIa32X64.dsc | 4 ++--
>>>> OvmfPkg/OvmfPkgIa32X64.fdf | 2 --
>>>> OvmfPkg/OvmfPkgX64.dsc     | 3 +--
>>>> OvmfPkg/OvmfPkgX64.fdf     | 2 --
>>>> OvmfPkg/OvmfXen.dsc        | 3 +--
>>>> OvmfPkg/OvmfXen.fdf        | 2 --
>>>> 8 files changed, 5 insertions(+), 16 deletions(-)
>>> 
>> 
>> Laszlo,
>> 
>> Thanks for the feedback. 
>> 
>> Can I ask that you go to https://www.tianocore.org, click on How to Contribute and point me at the chain of links I did not follow, if I missed it tit is likely due to too many links and too much information being vended.
> 
> You didn't miss anything, starting from that page, I think. I don't
> remember ever clicking "How to Contribute" on that page. :)
> 
> In fact if I grep the current edk2-wiki project source, at commit
> de8fae02bbcc ("Add acknowledgements page", 2020-05-21), I find no
> references to "SetupGit.py".
> 
>> When people are starting out we should vend them the instructions that work and let them opt in to learning more.
> 
> "Working instructions" is a moving target. When I wrote the unkempt
> guide, it was serious work, I had to set aside resources. When we
> changed the workflow to replace "git-push" (by maintainers) with github
> PRs (to trigger CI), documenting that was again serious work (for Mike
> -- Mike updated both the official workflow article and the unkempt
> guide, as I couldn't volunteer for the latter).
> 
> The more documentation we add, the larger the burden to update them
> grows. It's an on-going commitment. Given the constant scarcity of
> developer (and reviewer) cycles, we can only choose between:
> 
> - "code, plus more or less up-to-date docs", and
> - "code, plus no docs".
> 
> Leif wrote SetupGit.py in the first place to save people the effort of
> going through some of the unkempt guide steps.
> 
> If we decide that no SetupGit.py (or similar utilities) should be
> written without documenting them in the wiki, that won't force
> contributors to document their workflow-related contributions; it will
> cause them to not writing the utilities in the first place.
> 
> Open source development communities teach contributors the workflow by
> doing. For example, I have contributed to two open source projects that
> are *exclusively* managed on github.com, using "github native" pull
> requests and such. (Namely, openssl, and "p11-glue/p11-kit".) I'm the
> kind of guy that very carefully reads the documentation first, and
> starts pushing the buttons only second, and I *still* got wrong my
> initial contributions to both projects.
> 
> With OpenSSL, I missed details of how review worked and details about
> the CLA. Maintainers taught me those bits on the PR, while they were
> reviewing my code.
> 
> With p11-kit, I had missed that I was expected to write a unit test at
> once, for the new code. Maintainers pointed that out in my PR.
> 
> I posit that virtually no up-to-date technical documentation exists
> unless an organization treats that documentation as a *product* (with
> its own resource allocations, technical writers, subject matter expert
> reviewers, project managers, and so on).
> 
>> 
>>> (1) Please run "BaseTools/Scripts/SetupGit.py" in your edk2 clone,
>>> because right now, the patch is formatted/posted with too many CR
>>> characters.
>>> 
>> 
>> I filed https://bugzilla.tianocore.org/show_bug.cgi?id=2767 since I passed PatchCheck.py but did not run  BaseTools/Scripts/SetupGit.py
> 
> Thanks.
> 
> For the record: I didn't reject your contribution (I'm very happy you
> posted a patch series); instead, I asked for an update.
> 

Lazlo,

I understand that is just a tooling issue on my side. Sorry for my delay in fixing up the patches but I have some higher priority work PRs I need to get resolved and that is delaying me fixing up the patch set. 

At my work we have a tradition of trying to update our documentation when we onboard new people as that new set of eyes helps point out the small things that could have saved people a lot of time.  So I'm not so much complaining, but just trying to help. 

> In particular, the number of empty lines inserted into the DSC files is
> not consistent across the OVMF DSC files, and that fact has nothing to
> do with git configuration -- it would need fixing identically even if
> the series had been submitted via a github.com PR.
> 

My brain is a little dyslexic so I tend to miss symmetry things like this when I review my own work, but that is why we review the code. 

Thanks,

Andrew Fish


> Thanks
> Laszlo
> 
> 
> 
>