Replace Hard-coded Paths with Already Existing Variable

[LanceMcCarthy]

Problematic

At the end of the YAML for the continuous integration example, the upload-artifact step path uses a hard coded folder path:

https://github.com/microsoft/github-actions-for-desktop-apps/blob/d0fbf5cbc39057dea30244fb7ef6c5ae9c3b1ab8/.github/workflows/ci.yml#L93

Recommended

I believe this should be using the Wap_Project_Directory variable plus "AppPackages" subfolder instead.

Snippet

# Upload the MSIX package: https://github.com/marketplace/actions/upload-artifact
- name: Upload build artifacts
  uses: actions/upload-artifact@v1
  with:
    name: MSIX Package
    path: '$env:Wap_Project_Directory\AppPackages\'

[Edit] - Added the subfolder name to the variable. I'm not sure that's how to concatenate the variable with a string in GitHub actions yet, but you get the point

[LanceMcCarthy]

Following up again as there are similar inconsistencies in the CD action as well.

See the hard coded values on these two lines https://github.com/microsoft/github-actions-for-desktop-apps/blob/d0fbf5cbc39057dea30244fb7ef6c5ae9c3b1ab8/.github/workflows/cd.yml#L175-L176.

Though it does properly use the variable in the Create Archive step https://github.com/microsoft/github-actions-for-desktop-apps/blob/d0fbf5cbc39057dea30244fb7ef6c5ae9c3b1ab8/.github/workflows/cd.yml#L153

[edwardskrod]

I remember investigating this when I was creating the workflows. The problem is that we can't add an environment variable to a jobs..steps.with statement.

We can define the project directory name as a matrix variable in the CD workflow, but this starts to get repetitive:

matrix:
        channel: [Dev, Prod_Sideload, Prod_Store]
        targetPlatform: [x86, x64]
        include:

          # includes the following variables for the matrix leg matching Dev
          - channel: Dev
            WapProjectDir: MyWpfApp.Package
            ChannelName: Dev
...

          # includes the following variables for the matrix leg matching Prod_Sideload
          - channel: Prod_Sideload
            WapProjectDir: MyWpfApp.Package
            Configuration: Release

We would consume it like this:

    # Upload release asset:   https://github.com/actions/upload-release-asset
    - name: Update release asset
      id: upload-release-asset
      uses: actions/upload-release-asset@v1
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      with:
        upload_url: ${{ steps.create_release.outputs.upload_url }}  
        asset_path: ${{ matrix.AppPackagesPath }}\AppPackages\AppPackages.zip

For the CI workflow, I would need to create matrix variables.

      matrix:
        targetplatform: [x86, x64]
        include:

          # includes the following variables for the matrix leg matching x86
          - targetplatform: x86
            WapProjectDir: MyWpfApp.Package

            # includes the following variables for the matrix leg matching x86
          - targetplatform: x64
            WapProjectDir: MyWpfApp.Package

What do you think? I tend to think it's a little overly complicated.

[LanceMcCarthy]

Since you already need the matrix variable for channel name, it wouldn't be too much fluff to add another variable. However, the who point of a variable is that it's value is different, so it defeats the purpose to have the same value in each matrix.

So instead, you could just add a comment where things are hard-coded. For example:

 # We need to use full path here because job variable is not in scope
 path: MyWpfApp.Package\AppPackages\ 

That way, developers who might clone/copy from this repo wont get hung up on a path that doesn't exist in their app (you know folks are going to copy/paste without drilling deeper into each value).

[lindexi]

[lindexi]

@LanceMcCarthy I think you are right, see https://github.com/dotnet-campus/TranslationTool/runs/504675788

[edwardskrod]

Closed by https://github.com/microsoft/github-actions-for-desktop-apps/pull/26