Add several syntactical, functional logical improvements #11

aleks-ivanov · 2021-05-20T12:30:44Z

No description provided.

aleks-ivanov · 2021-05-20T12:37:31Z

.github/workflows/ci-cd.yml

@@ -1,14 +1,13 @@
 name: CI/CD Pipeline

-on: [push, pull_request, workflow_dispatch]
+on: [ push, pull_request, workflow_dispatch ]


Cosmetic change for better visibility.

aleks-ivanov · 2021-05-20T12:37:55Z

.github/workflows/ci-cd.yml


 jobs:
  ci:
    name: Continuous Integration
    runs-on: ubuntu-latest
    outputs:
      latest_version: ${{ steps.tag_generator.outputs.new_version }}
-      is_default_branch: ${{ steps.conditionals_handler.outputs.is_default_branch }}


Removed unnecessary value.

aleks-ivanov · 2021-05-20T12:38:48Z

.github/workflows/ci-cd.yml

@@ -23,7 +22,7 @@ jobs:
        shell: pwsh
        run: |
          # Get default branch
-          $repo = 'microsoft/OpenAPI.NET'
+          $repo = "${{ github.repository }}"


Replace static value with more elegant dynamic value - ${{ github.repository }}.

aleks-ivanov · 2021-05-20T12:40:03Z

.github/workflows/ci-cd.yml

          Write-Output "::set-output name=is_default_branch::$(echo $isDefaultBranch)"
+          Write-Output "::set-output name=is_push::$(echo $isPush)"
+          Write-Output "::set-output name=is_push_to_default_branch::$(echo $isPushToDefaultBranch)"


added two more conditional statements:

is_push = when the workflow is triggered by a push commit

is_push_to_default_branch = when the workflow is triggered by the a push commit in the default branch

aleks-ivanov · 2021-05-20T12:41:51Z

.github/workflows/ci-cd.yml

@@ -46,7 +56,7 @@ jobs:
          token: ${{ secrets.GITHUB_TOKEN }}
          fetch-depth: 0

-      - if: steps.conditionals_handler.outputs.is_default_branch == 'true'
+      - if: steps.conditionals_handler.outputs.is_push_to_default_branch == 'true'


Used the newly added is_push_to_default_branch condition, instead of the is_default_branch, to further restrict the new tag generating step to only execute when a push commit to the default branch is made.

aleks-ivanov · 2021-05-20T12:43:26Z

.github/workflows/ci-cd.yml

          $gitNewVersion = if ("${{ steps.tag_generator.outputs.new_version }}") {"${{ steps.tag_generator.outputs.new_version }}"} else {$null}
-          $projectCurrentVersion = ([xml](Get-Content .\src\Microsoft.OpenApi\Microsoft.OpenApi.csproj)).Project.PropertyGroup.Version
+          $projectCurrentVersion = ([xml](Get-Content -Path .\src\Microsoft.OpenApi\Microsoft.OpenApi.csproj)).Project.PropertyGroup.Version


Add argument as best practice. (writing scripts in PowerShell is all cmdlets and arguments should be written and with their full name for optimal legibility)

aleks-ivanov · 2021-05-20T12:56:09Z

.github/workflows/ci-cd.yml

-          # New-Item Artifacts/NuGet -ItemType Directory
-          # Get-ChildItem Artifacts/*.nupkg | Move-Item -Destination "Artifacts/NuGet"
+          # New-Item -Name Artifacts/NuGet -ItemType Directory
+          # Get-ChildItem -Path Artifacts/ -Filter *.nupkg | Move-Item -Destination Artifacts/NuGet


Add arguments as best practice.

aleks-ivanov · 2021-05-20T12:57:32Z

.github/workflows/ci-cd.yml

+            dotnet test $PSItem.FullName `
+            --configuration Release `
+            --logger "trx;LogFileName=$fileBaseName.trx;verbosity=normal" `
+            --results-directory TestResults/


Replaced static array of test project locations with a more elegant solution.
Add test logger, which generates test results with normal verbosity in the root/TestResults/ folder.

aleks-ivanov · 2021-05-20T12:57:51Z

.github/workflows/ci-cd.yml

+        with:
+          name: TestResults
+          path: TestResults/
+


Upload test results as pipeline artifact.

aleks-ivanov · 2021-05-20T12:59:55Z

.github/workflows/ci-cd.yml

@@ -103,7 +112,7 @@ jobs:
      #     path: Artifacts/NuGet/

  cd:
-    if: needs.ci.outputs.is_default_branch == 'true' && needs.ci.outputs.latest_version != ''
+    if: needs.ci.outputs.latest_version != ''


Removed is_default_branch condition, because it is logically unnecessary, since the step that dictates if the other condition needs.ci.outputs.latest_version is true or false, only executes when the workflow is triggered by a push commit in the default branch.

aleks-ivanov · 2021-05-20T13:01:59Z

.github/workflows/ci-cd.yml

-      #     Get-ChildItem NuGet/*.nupkg | ForEach-Object {
-      #       nuget push $PSItem `
+      #     Get-ChildItem -Path NuGet/ -Filter *.nupkg | ForEach-Object {
+      #       nuget push $PSItem.FullName `


Add .FullName to ensure the command always takes the file path as value.

aleks-ivanov · 2021-05-20T13:02:15Z

.github/workflows/ci-cd.yml

@@ -120,8 +129,8 @@ jobs:
      #   continue-on-error: true
      #   shell: pwsh
      #   run: |
-      #     Get-ChildItem NuGet/*.nupkg | ForEach-Object {
-      #       nuget push $PSItem `
+      #     Get-ChildItem -Path NuGet/ -Filter *.nupkg | ForEach-Object {


Add arguments as best practice

aleks-ivanov · 2021-05-20T13:16:00Z

.github/workflows/ci-cd.yml

-            -c Release # `
-            # -o $env:ARTIFACTS_FOLDER `
-            # /p:Version=$projectNewVersion 
+          Get-ChildItem -Path src/ -Filter *.csproj -Exclude *Workbench* -File -Recurse | ForEach-Object {


Replace static array of project locations with a more elegant solution.

aleks-ivanov added 3 commits May 20, 2021 12:37

add several improvements and simplifications

44ff419

add a few script improvements

89725d9

add tests logger and upload results as artifact

04952d5

aleks-ivanov commented May 20, 2021

View reviewed changes

.github/workflows/ci-cd.yml

with:

name: TestResults

path: TestResults/

Copy link

Collaborator Author

aleks-ivanov May 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upload test results as pipeline artifact.

aleks-ivanov commented May 20, 2021

View reviewed changes

aleks-ivanov added 2 commits May 20, 2021 16:09

add path to source project files search in build step

fa6afee

add path to project files search in test step

5766fc2

aleks-ivanov commented May 20, 2021

View reviewed changes

aleks-ivanov merged commit 91e27bd into pipeline-foundation:feature/improve-ci-cd May 20, 2021

aleks-ivanov deleted the feature/improve-ci-cd branch June 21, 2021 09:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add several syntactical, functional logical improvements #11

Add several syntactical, functional logical improvements #11

aleks-ivanov commented May 20, 2021

aleks-ivanov May 20, 2021

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021

aleks-ivanov May 20, 2021

aleks-ivanov May 20, 2021

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021

Add several syntactical, functional logical improvements #11

Add several syntactical, functional logical improvements #11

Conversation

aleks-ivanov commented May 20, 2021

aleks-ivanov May 20, 2021

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021 • edited Loading

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021 • edited Loading

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021 • edited Loading

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021 • edited Loading

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021 • edited Loading

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021 • edited Loading

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021 • edited Loading

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021 • edited Loading

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021

Choose a reason for hiding this comment

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading

aleks-ivanov May 20, 2021 •

edited

Loading