Re: [edk2-devel] [Patch wiki v2] EDK II CI: Update Phase 1 details and admin settings

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Lazlo,

One comment below.  I have made updates based on the rest
of your feedback.

Mike

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, December 3, 2019 9:08 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> <liming.gao@intel.com>; Andrew Fish <afish@apple.com>;
> Leif Lindholm <leif.lindholm@linaro.org>
> Subject: Re: [edk2-devel] [Patch wiki v2] EDK II CI:
> Update Phase 1 details and admin settings
> 
> On 12/02/19 18:56, Michael D Kinney via Groups.Io
> wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2315
> >
> > Update wiki pages providing details of the Phase 1
> > EDK II Continuous Integration features and links to
> all
> > supporting documentation.  Also provide notes for
> admins
> > to configure GitHub, Azure Pipelines, and Mergify.
> >
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> > ---
> >  ...I-Continuous-Integration-Administration.md | 100
> ++++++++++++++++++
> >  EDK-II-Continuous-Integration.md              |  75
> ++++++++++---
> >  2 files changed, 162 insertions(+), 13 deletions(-)
> >  create mode 100644 EDK-II-Continuous-Integration-
> Administration.md
> >
> > diff --git a/EDK-II-Continuous-Integration-
> Administration.md b/EDK-II-Continuous-Integration-
> Administration.md
> > new file mode 100644
> > index 0000000..669d28a
> > --- /dev/null
> > +++ b/EDK-II-Continuous-Integration-Administration.md
> > @@ -0,0 +1,100 @@
> > +# EDK II Continuous Integration Administration
> > +
> > +## Configuring edk2 repository
> > +
> > +1) Azure Pipelines Configuration Steps Part I
> > +   1) Goto https://dev.azure.com/tianocore
> > +   2) Create new project in TianoCore org called
> `edk2-ci`
> > +   3) Disable Boards
> > +   4) Disable Repositories
> > +
> > +2) GitHub Configuration Steps Part I
> > +   1) Goto https://github.com/tianocore
> > +   2) Select Settings
> > +   3) Select Installed GitHub Apps
> > +   4) Select Azure Pipelines -> Configure
> > +      * Enable `tianocore/edk2` repository
> > +      * Redirects to Azure Pipelines login.  Select
> TianoCore org and
> > +        `edk2-ci` project to complete link and
> authentication between Azure
> > +        Pipelines TianoCore organization and GitHub
> TianoCore organization.
> > +
> > +3) Azure Pipelines Configuration Steps Part II
> > +   1) Goto https://dev.azure.com/tianocore/edk2-
> ci/_build
> > +   2) Create New Pipeline called `Ubuntu GCC5 CI`
> for post commit checks
> > +      * YAML: Select file
> https://github.com/tianocore/edk2/blob/master/.azurepip
> elines/Ubuntu-GCC5.yml
> > +      * Name: Set to `Ubuntu GCC5 CI`
> > +      * Triggers:
> > +        * CI - No changes
> > +        * PR - Override + Disable
> > +      * Variables: No changes
> > +      * Save pipeline changes
> > +      * Run pipeline and verify all checks pass
> > +   3) Create New Pipeline called `Windows VS2019 CI`
> for post commit checks
> > +      * YAML: Select file
> https://github.com/tianocore/edk2/blob/master/.azurepip
> elines/Windows-VS2019.yml
> > +      * Name: `Windows VS2019 CI`
> > +      * Triggers
> > +        * CI - No changes
> > +        * PR - Override + Disable
> > +      * Variables: No changes
> > +      * Save pipeline changes
> > +      * Run pipeline and verify all checks pass
> > +   4) Create New Pipeline called `Ubuntu GCC5 PR`
> for pre commit checks
> > +      * YAML: Select file
> https://github.com/tianocore/edk2/blob/master/.azurepip
> elines/Ubuntu-GCC5.yml
> > +      * Name: `Ubuntu GCC5 PR`
> > +      * Triggers
> > +        * CI - Override + Disable
> > +        * PR - No changes
> > +      * Variables: No changes
> > +      * Save pipeline changes
> > +      * Run pipeline.  Pipeline will fail because it
> requires a GitHub PR.  Must
> > +        run for pipeline name to show up in GitHub
> Branch Protection Rules.
> > +   5) Create New Pipeline called `Windows VS2019 PR`
> for pre commit checks
> > +      * YAML: Select file
> https://github.com/tianocore/edk2/blob/master/.azurepip
> elines/Windows-VS2019.yml
> > +      * Name: `Windows VS2019 PR`
> > +      * Triggers
> > +        * CI - Override + Disable
> > +        * PR - No changes
> > +      * Variables: No changes
> > +      * Save pipeline changes
> > +      * Run pipeline.  Pipeline will fail because it
> requires a GitHub PR.  Must
> > +        run for pipeline name to show up in GitHub
> Branch Protection Rules.
> > +   6) Add PatchCheck pipeline for pre commit checks
> > +      * YAML: Select file
> https://github.com/tianocore/edk2/blob/master/.azurepip
> elines/Ubuntu-PatchCheck.yml
> > +      * Name: `tianocore.PatchCheck`
> > +      * Triggers
> > +        * CI - Override + Disable
> > +        * PR - No changes
> > +      * Variables: No changes
> > +      * Save pipeline changes
> > +      * Run pipeline.  Pipeline will fail because it
> requires a GitHub PR.  Must
> > +        run for pipeline name to show up in GitHub
> Branch Protection Rules.
> > +
> > +4) GitHub Configuration Steps Part II
> > +   1) Goto https://github.com/tianocore/edk2
> > +   2) Select Settings
> > +   3) Select  Branches
> 
> Double space character.
> 
> > +   4) Select Branch Protection Rules
> > +   5) Select `master` -> Edit
> > +      * Enable `Require status checks to pass before
> merging`
> > +      * Enable `Require branches to be up to date
> before merging`
> > +      * Enable `Windows VS2019 PR` check
> > +      * Enable `Ubuntu GCC5 PR` check
> > +      * Enable `tianocore.PatchCheck` check
> > +   6) Goto https://github.com/tianocore
> > +   7) Select Settings
> > +   8) Select Installed GitHub Apps
> > +   9) Select Mergify -> Configure
> > +      * Enable tianocore/edk2 repo
> > +
> > +5) Update Status Badge Links
> > +   1) Goto https://dev.azure.com/tianocore/edk2-
> ci/_build
> > +   2) Select `Windows VS2019 CI`
> > +      * Use '...' menu in upper right and select
> Status badge
> > +      * Copy Sample markdown
> > +      * Update links in Build Status section of
> https://github.com/tianocore/edk2/blob/master/Readme.md
> > +   3) Select `Ubuntu GCC5 CI`
> > +      * Use '...' menu in upper right and select
> Status badge
> > +      * Copy Sample markdown
> > +      * Update links in Build Status section of
> https://github.com/tianocore/edk2/blob/master/Readme.md
> > +   4) Submit changes to Readme.md for EDK II code
> review and submit GitHub PR to
> > +      test personal build and Mergify commit once
> review passes.
> > diff --git a/EDK-II-Continuous-Integration.md b/EDK-
> II-Continuous-Integration.md
> > index bfe8f30..3b0e573 100644
> > --- a/EDK-II-Continuous-Integration.md
> > +++ b/EDK-II-Continuous-Integration.md
> > @@ -1,23 +1,72 @@
> > -# EDK II Continuous Integration Proposal
> > +# EDK II Continuous Integration
> >
> > -# Phase 1 (edk2 repository only)
> > -* Remove write access to edk2 repo
> > -* EDK II Maintainers use GitHub Pull Request instead
> of push
> > -* Only accept PR from EDK II Maintainers.  Reject
> all other PRs.
> > -* Run basic Pre-commit checks
> > -* If all Pre-commit checks pass, then auto commit
> changes
> > -* If any Pre-commit check fails, then notify
> submitter
> > -* Limit pre-commit check execution time to 10
> minutes
> > -* Provide on-demand builds to EDK II Maintainers
> > +Summary of pre-commit and post-commit Continuous
> Integration services that
> > +improve the quality of commits made to EDK II
> repositories.  The sections below
> > +list the Continuous Integration services that are
> active and plans for future
> > +enhancements and extensions to these services.
> >
> > -## Proposed Pre-Commit Checks in Phase 1
> > -* PatchCheck.py
> > +## Phase 1 (edk2 repository only) - Activated
> November 11, 2019
> > +
> > +1) Use a combination of GitHub, Azure Pipelines,
> Mergify, and edk2-pytool features.
> > +   * GitHub Pull Requests + Labels, Branch
> Protections,  Notifications
> > +   * Mergify Pull Request Rules with auto commit if
> all checks pass
> > +   * 3 pre-commit jobs in Azure Pipelines
> (PatchCheck, Windows/VS, Linux/GCC).
> > +     Goal is to complete all pre-commits check in
> under 10 minutes.
> > +   * 2 post-commit jobs in Azure Pipelines
> (Windows/VS, Linux/GCC).  Post commit
> > +     status provided at top of `edk2/master`
> [Readme.md](https://github.com/tianocore/edk2/blob/mast
> er/Readme.md).
> > +   * [EDK II Pytool
> Library](https://github.com/tianocore/edk2-pytool-
> library)
> > +   * [EDK II Pytool
> Extensions](https://github.com/tianocore/edk2-pytool-
> extensions)
> > +   * [TianoCore Bugzilla
> #2315](https://bugzilla.tianocore.org/show_bug.cgi?id=2
> 315)
> > +   * Original RFC Proposals
> > +     - https://edk2.groups.io/g/rfc/message/93
> > +     - https://edk2.groups.io/g/devel/message/46607
> > +2) Enable the following pre-commit checks
> > +   *
> [PatchCheck](https://github.com/tianocore/edk2/blob/mas
> ter/BaseTools/Scripts/PatchCheck.py)
> > +   *
> [CharEncodingCheck](https://github.com/tianocore/edk2/t
> ree/master/.pytool/Plugin/CharEncodingCheck)
> > +   *
> [CompilerPlugin](https://github.com/tianocore/edk2/tree
> /master/.pytool/Plugin/CompilerPlugin)
> > +   *
> [DependencyCheck](https://github.com/tianocore/edk2/tre
> e/master/.pytool/Plugin/DependencyCheck)
> > +   *
> [DscCompleteCheck](https://github.com/tianocore/edk2/tr
> ee/master/.pytool/Plugin/DscCompleteCheck)
> > +   *
> [GuidCheck](https://github.com/tianocore/edk2/tree/mast
> er/.pytool/Plugin/GuidCheck)
> > +   *
> [LibraryClassCheck](https://github.com/tianocore/edk2/t
> ree/master/.pytool/Plugin/LibraryClassCheck)
> > +   *
> [SpellCheck](https://github.com/tianocore/edk2/tree/mas
> ter/.pytool/Plugin/SpellCheck)
> 
> Not too important, but I guess I might as well raise
> it:
> 
> in external references (URLs) to specific files, I
> prefer to capture
> commit hashes, not branch names. That's because a
> commit hash is
> forever, but a branch name is a moving target. So we
> could replace
> "master" with the "value" of "master" at this time.
> 
> This is particularly helpful when a file is renamed
> later on, in the
> edk2 repository. With "master" in the external URLs,
> those URLs break.
> With a commit hash, the links continue working -- they
> won't be 100%
> up-to-date, of course, but they will reflect reality as
> it was at the
> time the wiki article was written.
> 
> Again, not too important.
> 
> > +3) TianoCore EDK II Maintainers Team permissions
> reduced from 'Write" to "Triage"
> > +4) EDK II Maintainers must use GitHub pull request
> with 'push' label to request
> > +   a branch to be strict rebase merged into
> `edk2/master`.  If all checks pass,
> 
> Perhaps replace "strict rebase" with "strict fast-
> forward"?

Please look at these descriptions:

  https://doc.mergify.io/actions.html#git-merge-workflow-and-mergify-equivalent-configuration

We are currently using strict: true, method: rebase

  https://github.com/tianocore/edk2/blob/master/.mergify/config.yml

Let me know if you what is the best way to describe this.
I am currently using terms for the Mergify settings and not
the git terminology.

> 
> > +   then the patches in the pull request are
> automatically added to `edk2/master`.
> > +   If any checks fails, then email notifications are
> sent and details of the
> 
> Suggest replacing "checks fails" with "check fails" or
> "checks fail".
> 
> > +   failure are available through Azure Pipelines
> test results.
> > +5) Personal builds available to all EDK II
> developers using a GitHub pull
> > +   request without the 'push' label set.  If all
> checks pass, then a notification
> > +   email is sent and the pull request is closed.  If
> any checks fails, then
> > +   email notifications are sent and the details of
> the failure are available
> > +   through Azure Pipelines test results.
> > +6) GitHub References
> > +   * [GitHub](https://github.com/)
> > +   * [GitHub
> Labels](https://help.github.com/en/github/managing-
> your-work-on-github/about-labels)
> > +   * [GitHub Protected
> Branches](https://help.github.com/en/github/administeri
> ng-a-repository/about-protected-branches)
> > +   * [GitHub
> Notifications](https://help.github.com/en/github/receiv
> ing-notifications-about-activity-on-github/about-
> notifications)
> > +   * [Watch a GitHub
> repository](https://help.github.com/en/github/receiving
> -notifications-about-activity-on-github/watching-and-
> unwatching-repositories)
> > +   * [Create a GutHub
> fork](https://help.github.com/en/github/getting-
> started-with-github/fork-a-repo)
> > +   * [Create a GitHub pull
> request](https://help.github.com/en/github/collaboratin
> g-with-issues-and-pull-requests/creating-a-pull-
> request)
> > +7) HUB Command line Utility to perform GitHub
> operations
> > +   * [HUB
> Releases](https://github.com/github/hub/releases)
> > +   * [HUB Usage](https://hub.github.com/hub.1.html)
> > +8) Azure Pipelines References
> > +   * [Azure Piplelines GitHub
> App](https://github.com/marketplace/azure-pipelines)
> > +   * [Azure Pipelines TianoCore edk2-ci
> Project](https://dev.azure.com/tianocore/edk2-ci)
> > +   * [Azure Pipelines TianoCore edk2-ci
> Pipelines](https://dev.azure.com/tianocore/edk2-
> ci/_build)
> > +   * [Azure
> Pipelines](https://azure.microsoft.com/en-
> us/services/devops/pipelines)
> > +   * [Azure Pipelines YAML
> Schema](https://docs.microsoft.com/en-
> us/azure/devops/pipelines/yaml-schema)
> > +9) Mergify References
> > +   * [Mergify GitHub
> App](https://github.com/apps/mergify)
> > +   * [Mergify Documentation](https://doc.mergify.io)
> > +
> > +[[EDK II Continuous Integration Administration]]
> 
> Hmm, this kind of "naked" link surprised me, but it
> does look nice in
> the rendered view. OK.
> 
> >
> >  ## Proposed Pre-Commit Checks in Phase 2
> >  * Verify Reviewed-by and Acked-by tags are present
> with correct maintainers
> >  * Verify no non-ASCII characters in modified files
> >  * Verify no binary files in set of modified files
> > -* Verify Package Dependency rules in modified files
> 
> This update seems to stick a bit out of the rest of
> this patch. Are we
> removing this item because it's already covered by
> DependencyCheck?
> 
> If so, can we mention that in the commit message?
> 
> >
> >  ## Proposed Pre-Commit Checks in Phase 3
> >  * Run ECC on modified files
> >
> 
> I only requested superficial changes. Thank you for
> writing all this up.
> Please address whatever points of mine you agree with.
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> Thanks!
> Laszlo