Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contrib-build from OpenMS/contrib release #55

Merged
merged 1 commit into from
May 6, 2024

Conversation

Arslan-Siraj
Copy link
Contributor

@Arslan-Siraj Arslan-Siraj commented May 6, 2024

User description

  • not needed abibuilder contrib anymore

PR Type

enhancement


Description

  • Updated the GitHub Actions workflow for building Windows executables to use GitHub's release feature for downloading dependencies.
  • This change includes the use of gh release download for fetching the contrib_build-Windows.tar.gz file and 7z for extraction, replacing the previous use of curl and tar.
  • Added GITHUB_TOKEN as an environment variable to authenticate GitHub CLI operations.

Changes walkthrough 📝

Relevant files
Enhancement
build-windows-executable-app.yaml
Update Windows Executable Build Process in GitHub Actions

.github/workflows/build-windows-executable-app.yaml

  • Added environment variable GITHUB_TOKEN for authentication.
  • Changed the method of downloading the contrib build using GitHub's
    release feature instead of a direct URL.
  • Replaced curl and tar commands with gh release download and 7z
    commands for handling the download and extraction of files.
  • +9/-5     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    - not needed abibuilder contrib anymore
    @codiumai-pr-agent-pro codiumai-pr-agent-pro bot added the enhancement New feature or request label May 6, 2024
    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Description updated to latest commit (331322c)

    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are limited to a single workflow file and involve straightforward modifications such as replacing the method of downloading and extracting files, and adding an environment variable.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    Possible Bug: The use of gh release download assumes that the GitHub CLI is pre-installed in the runner environment. If not, this could lead to a failure in executing the workflow.

    🔒 Security concerns

    No

    Copy link

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Add a conditional check to only download the contrib build when it's not already cached.

    It's recommended to use a conditional check to ensure that the contrib build is only
    downloaded when it's not already cached. This avoids unnecessary downloads and can save
    time and bandwidth.

    .github/workflows/build-windows-executable-app.yaml [65-75]

     - name: Load contrib build
    +  if: steps.cache-contrib-win.outputs.cache-hit != 'true'
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       run: |
         cd OpenMS/contrib
         gh release download -R OpenMS/contrib --pattern 'contrib_build-Windows.tar.gz'
         7z x -so contrib_build-Windows.tar.gz | 7z x -si -ttar
         rm contrib_build-Windows.tar.gz
         ls
     
    Best practice
    Use PowerShell commands for file operations to improve compatibility and readability.

    Consider using PowerShell commands for file operations on Windows to ensure compatibility
    and potentially improve error handling and readability.

    .github/workflows/build-windows-executable-app.yaml [73-74]

    -7z x -so contrib_build-Windows.tar.gz | 7z x -si -ttar
    -rm contrib_build-Windows.tar.gz
    +Expand-7Zip -ArchiveFileName contrib_build-Windows.tar.gz -TargetPath .
    +Remove-Item -Path contrib_build-Windows.tar.gz
     
    Verify the integrity of downloaded files before extraction.

    To ensure that the downloaded files are correctly handled, consider verifying the
    integrity of the downloaded archive before extracting it.

    .github/workflows/build-windows-executable-app.yaml [71-73]

     gh release download -R OpenMS/contrib --pattern 'contrib_build-Windows.tar.gz'
    -7z x -so contrib_build-Windows.tar.gz | 7z x -si -ttar
    +if (Test-Path -Path contrib_build-Windows.tar.gz) {
    +    7z x -so contrib_build-Windows.tar.gz | 7z x -si -ttar
    +}
    +else {
    +    Write-Error "Downloaded file does not exist."
    +}
     
    Security
    Use environment variables for secrets to enhance security.

    To enhance security, avoid directly embedding secrets in the workflow file. Instead, use
    environment variables set in the GitHub Actions environment.

    .github/workflows/build-windows-executable-app.yaml [66-67]

     env:
    -  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
    +  GITHUB_TOKEN: ${{ env.GITHUB_TOKEN }}
     
    Maintainability
    Add error handling for the file extraction process.

    Consider adding error handling for the extraction process to manage potential failures
    gracefully.

    .github/workflows/build-windows-executable-app.yaml [73]

    -7z x -so contrib_build-Windows.tar.gz | 7z x -si -ttar
    +$exitCode = 7z x -so contrib_build-Windows.tar.gz | 7z x -si -ttar
    +if ($exitCode -ne 0) {
    +    Write-Error "Failed to extract files."
    +    exit $exitCode
    +}
     

    @axelwalter axelwalter merged commit e2f082d into OpenMS:main May 6, 2024
    6 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants