-
Notifications
You must be signed in to change notification settings - Fork 53
🌱 analyzer + provider workflow artifacts #946
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
Conversation
WalkthroughThe 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 Changes
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Signed-off-by: David Zager <[email protected]>
5c537a3 to
ba04773
Compare
There was a problem hiding this 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 buildfails 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:
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: ...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 binariesUpdate 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
📒 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 buildcommand is portable across all platforms—it uses standardgo buildwithout platform-specific logic. However, the workflow setsGOOSandGOARCHenvironment 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:
- Remove the unused
GOOS/GOARCHenvironment variables from the build step, or update the Makefile to use them if cross-compilation is intended.- Verify that the artifact upload steps (lines 47–70) reference the correct paths where
make buildoutputs binaries:
- Root:
konveyor-analyzer,konveyor-analyzer-depexternal-providers/generic-external-provider/generic-external-providerexternal-providers/golang-dependency-provider/golang-dependency-providerexternal-providers/yq-external-provider/yq-external-providerexternal-providers/java-external-provider/java-external-provider
Make it possible to develop with
npm run collect-assets:devfrom konveyor/editor-extension (specifically in the distributed language extension feature branch).Summary by CodeRabbit