Skip to content

ci: add gradle caching and release workflow - #4

Merged
adimail merged 2 commits into
mainfrom
ci/workflows
Apr 4, 2026
Merged

adimail merged 2 commits into
mainfrom
ci/workflows

Conversation

@swarooppatilx

Copy link
Copy Markdown
Owner

No description provided.

@swarooppatilx
swarooppatilx requested a review from adimail April 4, 2026 18:46

@adimail adimail left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review: ci/workflows → main

build.yml

🔴 Wrong trigger — builds on every push, not just PRs

The workflow fires on both push and pull_request, meaning it runs on every
push to every branch. Since the intent is to catch issues on PRs, restrict it
to pull_request only.

🔴 No explicit test step

./gradlew build does run tests in most Gradle setups, but being explicit is
safer and more readable. Rename the build step to test and run ./gradlew test instead.

🟡 Artifact upload doesn't belong here

Uploading artifacts on every PR run is noisy and wasteful. The
capture build artifacts step should live only in release.yml.


release.yml

🟡 Use assemble instead of build

Tests already passed on the PR — re-running them at release time is redundant
and slow. Use assemble to just produce the JAR without re-running tests.


✅ What's Good

  • Tag pattern v* correctly scopes the release trigger
  • permissions: contents: write is correctly and minimally scoped
  • generate_release_notes: true is a nice touch
  • All actions pinned to major versions (@v4, @v2)

Summary

# File Issue Severity
1 build.yml Triggers on push — should be pull_request only 🔴
2 build.yml No explicit test step 🔴
3 build.yml Artifact upload on every PR is unnecessary 🟡
4 release.yml Use assemble not build 🟡

@adimail adimail left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@adimail adimail left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@adimail
adimail merged commit 59edca5 into main Apr 4, 2026
1 check passed
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