-
Notifications
You must be signed in to change notification settings - Fork 246
ci: fix release workflow #3027
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
ci: fix release workflow #3027
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
|
Claude finished @julienrbrt's task in 2m 12s —— View job Comprehensive Code ReviewI've completed a thorough review of the release workflow changes. Overall, the changes correctly implement Go module tagging conventions and make important improvements. Here's my detailed analysis: SummaryVerdict: ✅ LGTM with Minor Suggestions The PR successfully fixes the Go tagging issue by removing the hardcoded Detailed Review1. Code Quality & Architecture ✅Strengths:
Observations:
2. Error Handling & Resilience ✅Strengths:
Suggestions:
3. Security 🔒Strengths:
Improvement from Previous Version:
Note on Deleted goreleaser.yml: 4. Performance & Resource Efficiency ⚡Strengths:
Potential Optimization: - name: Build and push Docker image
uses: docker/build-push-action@v6
with:
context: .
file: ${{ needs.parse-tag.outputs.dockerfile }}
push: true
platforms: linux/amd64,linux/arm64
cache-from: type=gha
cache-to: type=gha,mode=max
tags: |
ghcr.io/${{ github.repository_owner }}/${{ needs.parse-tag.outputs.image-name }}:${{ needs.parse-tag.outputs.version }}
ghcr.io/${{ github.repository_owner }}/${{ needs.parse-tag.outputs.image-name }}:latest5. Testing & Reliability 🧪Strengths:
Concerns:
Suggestion - Add Test Job (optional but recommended): test:
name: Run Tests
runs-on: ubuntu-latest
needs: parse-tag
steps:
- uses: actions/checkout@v6
- uses: actions/setup-go@v6
with:
go-version-file: ${{ needs.parse-tag.outputs.app-path }}/go.mod
- name: Run tests
run: cd ${{ needs.parse-tag.outputs.app-path }} && go test ./...
build-and-push:
needs: [parse-tag, test] # Add test dependency
# ... rest of job6. Documentation & Developer Experience 📚Strengths:
Missing Documentation:
Suggestion: Add a section to the PR description: ## Breaking Changes
- Tags must now include the `apps/` prefix to match Go module paths
- ❌ Old: `testapp/v0.4.0`
- ✅ New: `apps/testapp/v0.4.0`
## Architectural Changes
- Removed `goreleaser.yml` workflow (consolidated into `release.yml`)
- Release process now only builds and pushes Docker images
- Binary releases via goreleaser are no longer automatedCritical Questions❓ Goreleaser RemovalThe deletion of
Questions for PR author:
❓ Tag Pattern Mismatch RiskThe tag pattern tags:
- "apps/*/v*.*.*" # Matches only one level deep
- "apps/*/*/v*.*.*" # Matches two levels deepThis would prevent accidental triggers from incorrectly placed tags. Security Improvements from Base✅ Major security wins:
Final RecommendationsMust Address:
Should Consider:
Nice to Have:
ConclusionThe core fix (removing Recommendation: Approve after addressing the goreleaser question and adding release notes about the breaking tag format change. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3027 +/- ##
==========================================
- Coverage 55.66% 55.59% -0.07%
==========================================
Files 116 116
Lines 11477 11477
==========================================
- Hits 6389 6381 -8
- Misses 4389 4395 +6
- Partials 699 701 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Overview
We must tag following the go tagging norm, so
git tag /path/to/go.mod, so we shouldn't prepend./appsto the path in the workflow: https://github.com/evstack/ev-node/actions/runs/21438410740/job/61734871603