Skip to content

Conversation

@djzager
Copy link
Member

@djzager djzager commented Oct 29, 2025

Make it possible to develop with npm run collect-assets:dev from konveyor/editor-extension (specifically in the distributed language extension feature branch).

Summary by CodeRabbit

  • Chores
    • Enhanced CI build and artifact management to support multiple OSes and architectures (Linux, macOS, Windows on x64 and ARM64).
    • Updated workflow tooling versions and added a cross-platform build step that produces platform-specific binaries.
    • Added per-target artifact uploads (including multiple executables), platform-aware naming (Windows .exe suffix) and conditional publishing.

@coderabbitai
Copy link

coderabbitai bot commented Oct 29, 2025

Walkthrough

The PR updates the GitHub Actions workflow to use an explicit include-based matrix of OS/arch targets, adds a platform-aware build step that sets GOOS/GOARCH and runs make, and adds three artifact upload steps that publish per-target binaries (with Windows .exe handling).

Changes

Cohort / File(s) Summary
Workflow matrix & build step
.github/workflows/pr-testing.yml
Replaced implicit runs-on matrix with an include-based matrix enumerating OS/ARCH combos and artifact_name; added "Build binaries" step that exports GOOS/GOARCH from the matrix and runs make build.
Artifact uploads
.github/workflows/pr-testing.yml
Added three conditional upload-artifact steps for analyzer-lsp, analyzer-lsp-deps, and external-providers with per-target artifact names (including commit SHA) and Windows .exe suffix logic; handles multiple provider executables for external-providers.
Actions versions
.github/workflows/pr-testing.yml
Bumped action versions: actions/checkout v3 → v5 and actions/setup-go v3 → v6.

Sequence Diagram(s)

sequenceDiagram
  participant GH as GitHub Actions
  participant Matrix as Matrix Evaluator
  participant Runner as Platform Runner
  participant Make as make
  participant Artifact as Upload Artifact

  GH->>Matrix: trigger job (include-based matrix)
  Matrix-->>Runner: pick OS/ARCH + artifact_name
  Runner->>Runner: export GOOS, GOARCH
  Runner->>Make: run `make build` (build binaries)
  Make-->>Runner: produce binaries (platform-specific, .exe on Windows)
  Runner->>Artifact: upload `analyzer-lsp` (artifact_name + SHA)
  Runner->>Artifact: upload `analyzer-lsp-deps` (artifact_name + SHA)
  Runner->>Artifact: upload `external-providers` (multiple executables)
  Artifact-->>GH: attach artifacts to workflow run
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect matrix entries for correct OS/ARCH and artifact_name mapping.
  • Verify GOOS/GOARCH environment setting and that make build produces expected artifacts per platform.
  • Confirm artifact path patterns, conditional .exe suffix logic, and that external-providers uploads include all intended executables.
  • Check updated action versions for any required option changes.

Poem

🐰 I hopped through YAML fields so bright,

built binaries by day and night,
across each OS my paws did sing,
artifacts bundled on a string,
a carrot for CI's delight 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title :seedling: analyzer + provider workflow artifacts directly aligns with the main changes in the changeset, which involve updating the GitHub Actions workflow to build and upload artifacts for the analyzer and provider binaries. The title is concise, clear, and specific enough for a reviewer scanning the history to understand that this PR introduces workflow artifact uploads for these components. While the changes include additional details like updating action versions and refactoring the matrix configuration, the title appropriately focuses on the primary objective without unnecessary verbosity.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@djzager djzager force-pushed the provider-artifacts branch from 5c537a3 to ba04773 Compare October 29, 2025 01:22
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
.github/workflows/pr-testing.yml (1)

47-70: Add error handling and validation for artifact uploads.

The three artifact upload steps reference specific binary paths but lack guards to ensure the files exist before upload. If make build fails or produces binaries at different paths than expected, the upload-artifact steps may error or silently skip, potentially masking build failures.

Consider one of the following approaches:

  1. Add if: conditions to each upload step to verify files exist or to conditionally run based on platform:

    - name: Upload analyzer-lsp binary
      if: success()  # or platform-specific conditions
      uses: actions/upload-artifact@v4
      with: ...
  2. Add a validation step before uploads to confirm expected binaries are present:

    - name: Verify build artifacts
      run: |
        [ -f "konveyor-analyzer${{ matrix.goos == 'windows' && '.exe' || '' }}" ] || exit 1
        # ... verify other binaries
  3. Update the upload step to use if-no-files-found: 'error' to explicitly fail if files are missing (upload-artifact v4 supports this).

      - name: Verify build artifacts
        run: |
          set -e
          echo "Checking for analyzer-lsp binary..."
          ls -la konveyor-analyzer${{ matrix.goos == 'windows' && '.exe' || '' }}
          echo "Checking for analyzer-lsp-deps binary..."
          ls -la konveyor-analyzer-dep${{ matrix.goos == 'windows' && '.exe' || '' }}
          echo "Checking for external-providers binaries..."
          ls -la external-providers/*/

      - name: Upload analyzer-lsp binary
        uses: actions/upload-artifact@v4
        with:
          name: analyzer-lsp.${{ matrix.artifact_name }}-${{ github.sha }}
          path: konveyor-analyzer${{ matrix.goos == 'windows' && '.exe' || '' }}
          retention-days: 30
          if-no-files-found: 'error'

      - name: Upload analyzer-lsp deps binary
        uses: actions/upload-artifact@v4
        with:
          name: analyzer-lsp-deps.${{ matrix.artifact_name }}-${{ github.sha }}
          path: konveyor-analyzer-dep${{ matrix.goos == 'windows' && '.exe' || '' }}
          retention-days: 30
          if-no-files-found: 'error'

      - name: Upload external-providers binaries
        uses: actions/upload-artifact@v4
        with:
          name: external-providers.${{ matrix.artifact_name }}-${{ github.sha }}
          path: |
            external-providers/generic-external-provider/generic-external-provider${{ matrix.goos == 'windows' && '.exe' || '' }}
            external-providers/golang-dependency-provider/golang-dependency-provider${{ matrix.goos == 'windows' && '.exe' || '' }}
            external-providers/yq-external-provider/yq-external-provider${{ matrix.goos == 'windows' && '.exe' || '' }}
            external-providers/java-external-provider/java-external-provider${{ matrix.goos == 'windows' && '.exe' || '' }}
          retention-days: 30
          if-no-files-found: 'error'
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9335564 and ba04773.

📒 Files selected for processing (1)
  • .github/workflows/pr-testing.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: test
  • GitHub Check: test (macos-latest, darwin, amd64, macos-x86_64)
  • GitHub Check: test (windows-latest, windows, amd64, windows-X64)
🔇 Additional comments (1)
.github/workflows/pr-testing.yml (1)

9-45: Remove unused GOOS/GOARCH environment variables; verify artifact upload paths match Makefile outputs.

The make build command is portable across all platforms—it uses standard go build without platform-specific logic. However, the workflow sets GOOS and GOARCH environment variables that the Makefile does not use. Since each matrix job runs on its target OS (ubuntu-latest, macos-latest, windows-latest), native compilation automatically produces correct binaries, making these variables redundant.

Recommended actions:

  1. Remove the unused GOOS/GOARCH environment variables from the build step, or update the Makefile to use them if cross-compilation is intended.
  2. Verify that the artifact upload steps (lines 47–70) reference the correct paths where make build outputs binaries:
    • Root: konveyor-analyzer, konveyor-analyzer-dep
    • external-providers/generic-external-provider/generic-external-provider
    • external-providers/golang-dependency-provider/golang-dependency-provider
    • external-providers/yq-external-provider/yq-external-provider
    • external-providers/java-external-provider/java-external-provider

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants