-
Notifications
You must be signed in to change notification settings - Fork 52
CASSSIDECAR-397 Add Missing checks in GitHub actions vs Cicle CI #310
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
The GitHub Actions CI workflow was missing several checks present in Circle CI. This adds: 1. Java 8 client unit tests - verifies client compatibility with Java 8 2. Deb package build/install - verifies Debian package builds and installs 3. RPM package build/install - verifies RPM package builds and installs 4. Docker build - verifies Docker image builds with Jib 5. Documentation build - verifies docs build with Asciidoctor Also includes the checkstyle job added earlier to catch style violations. This ensures GitHub Actions green status implies Circle CI will also pass.
CASSSIDECAR-397 Add checkstyle job to GitHub Actions CI
|
Thanks for the patch. Running actions now. |
|
looks like a flaky test failed |
|
Triggered re-run of the failed job. |
frankgh
left a comment
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.
Thanks for the patch! I have some minor suggestions.
.github/workflows/ci.yml
Outdated
| path: dtest-jars/ | ||
|
|
||
| - name: Run checkstyle | ||
| run: ./gradlew checkstyleMain checkstyleTest checkstyleTestFixtures checkstyleIntegrationTest --stacktrace |
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.
does it make sense to register a new task in the main build.gradle file to run all checkstyle tasks? I'm thinking something like this:
tasks.register('checkstyle') {
description = 'Runs all checkstyle tasks'
group = 'verification'
dependsOn tasks.withType(Checkstyle)
}
Then we can run ./gradlew checkstyle without having to list all targets
CHANGES.txt
Outdated
| @@ -1,5 +1,6 @@ | |||
| 0.3.0 | |||
| ----- | |||
| * Add Missing checks in GitHub actions vs Cicle CI (CASSSIDECAR-397) | |||
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.
CI changes don't need to be mentioned in CHANGES.txt, we can remove this
Simplifies checkstyle invocation and removes CI-only notes from CHANGES for consistency.
Address review feedback for CI checks
Co-authored-by: Francisco Guerrero <frank.guerrero@gmail.com>
* Remove unnecessary dtest-jars dependency from checkstyle job Checkstyle is a static analysis tool that runs directly on source files without needing compiled classes or Cassandra dependencies. Removing the dependency on build-dtest-jars allows checkstyle to run immediately in parallel, providing faster feedback on code style issues. * Restore dtest-jars dependency for checkstyle job The test-common:checkstyleTestFixtures task requires compiling testFixtures first, which depends on Cassandra dtest jars for import resolution. * Match CircleCI: use full build for Java 8 unit tests CircleCI runs './gradlew build' which relies on Gradle's Java version checking to skip server modules that require Java 11+. * Format run commands with pipe style for consistency
|
Triggering the actions again. |
frankgh
left a comment
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.
+1 looks good to me
sarankk
left a comment
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.
+1 Thanks for the patch, looks good to me
Summary
Adds missing CI jobs to GitHub Actions to ensure parity with Circle CI.
Changes