Skip to content

Ci - add build workflow - #28

Open
Tova-Rozovsky wants to merge 8 commits into
Rajath2005:mainfrom
Tova-Rozovsky:ci/add-build-workflow
Open

Ci - add build workflow#28
Tova-Rozovsky wants to merge 8 commits into
Rajath2005:mainfrom
Tova-Rozovsky:ci/add-build-workflow

Conversation

@Tova-Rozovsky

@Tova-Rozovsky Tova-Rozovsky commented Jun 28, 2026

Copy link
Copy Markdown

Key Changes:

Automated Build Pipeline: Created a GitHub Actions workflow to automate the compilation of the Java source code.

CI Configuration: Implemented path filtering to trigger the build process only when changes are made to Java source files (src//*.java), ensuring efficient resource usage.

Project Documentation: Integrated a build status badge into the README.md to provide immediate feedback on the project's build health.

Closes #24

@Rajath2005 Rajath2005 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for your contribution! I appreciate the effort you've put into setting up a GitHub Actions CI workflow. Automating the compilation process is a great addition and will help maintain the project's quality as it grows.

After reviewing the changes, I found one issue that needs to be addressed before this PR can be merged:

Required Change

  • The build status badge added to the README.md currently points to your fork:

    https://github.com/Tova-Rozovsky/SmartCityApp/actions/workflows/ci.yaml
    

    Since this repository is Rajath2005/SmartCityApp, the badge should reference the main repository instead so it reflects the actual build status of the project after merging. Please update the badge URL accordingly.

Optional Improvement

  • Instead of installing Java manually using apt-get, consider using the official actions/setup-java GitHub Action. It is the recommended approach by GitHub, provides better caching, and simplifies the workflow configuration.

Overall, this is a valuable contribution. Once the README badge is updated to point to the main repository, I'll be happy to review the updated changes again.

Thanks again for contributing to SmartCityApp!

@Tova-Rozovsky

Copy link
Copy Markdown
Author

Hi @Rajath2005,
I have updated the README badge and the CI workflow according to your feedback.
Thank you!

@Rajath2005 Rajath2005 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the update and for addressing the previous feedback! I appreciate the improvements to the workflow and the README badge.

I noticed one remaining issue before we can merge:

  • The path filter currently uses src/**.java. Since the project's Java files are organized in nested packages (for example, src/com/smartcity/...), this should be updated to src/**/*.java so that changes to Java files in any subdirectory correctly trigger the workflow.

Additionally, as a small cleanup, the Handle Failure step can be removed because GitHub Actions automatically marks the job as failed if javac exits with a non-zero status.

Once the path pattern is updated, I'll be happy to review it again. Thanks for the contribution!

@Tova-Rozovsky

Copy link
Copy Markdown
Author

Hi @Rajath2005, thank you for your patience! I believe I have updated the workflow according to your feedback (using src/**/*.java and removing the failure handling step).

I'm currently seeing that the branch is up to date, but if you still see any issues in the configuration, please let me know or point out exactly what might be missing. I am happy to make further adjustments if needed to get this merged!

@Rajath2005 Rajath2005 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The badges added to the README.md look good, but there are a few significant issues with the newly added GitHub Actions workflow (.github/workflows/ci.yaml) that should be addressed before merging:

1. Java Version Mismatch

The workflow configures the runner to use JDK 11, but if you check the project's pom.xml, maven.compiler.source and maven.compiler.target are set to 21. The workflow will likely fail or cause issues down the line.
Fix: Change java-version: '11' to java-version: '21'.

2. Incorrect Build Mechanism (Bypassing Maven)

The workflow uses a shell script to manually find and compile java files (find src -name "*.java" > sources.txt and javac @sources.txt). Because this is a Maven project, this approach is incorrect for several reasons:

  • It completely ignores dependencies defined in pom.xml (like mysql-connector-j), which will cause compilation errors if those dependencies are used in the code.
  • It bypasses the maven-checkstyle-plugin configured in the pom.xml (which is meant to fail the build on linting errors).
    Fix: Replace the manual javac step with standard Maven commands. For example:
      - name: Build with Maven
        run: mvn --batch-mode clean verify

(Note: using verify is recommended here as it will trigger the Checkstyle validation configured in your POM).

3. Missing cache for Maven (Optional but Recommended)

When switching to a Maven build, the setup-java action can automatically cache dependencies to speed up subsequent runs.
Fix: Add cache: 'maven' to the setup-java step.

4. Overly Restrictive Trigger Paths

The workflow currently only triggers if a file in src/**/*.java is modified. This means if someone updates a dependency in pom.xml, or updates checkstyle.xml, the CI will not run to verify if the build is still healthy.
Fix: Consider removing the paths: filters entirely so the CI runs on all PRs to main, or at least explicitly add pom.xml and checkstyle.xml to the paths list.


Suggested ci.yaml Replacement

Here is a suggested replacement for .github/workflows/ci.yaml:

name: Java CI

on:
  push:
    branches: [ main ]
  pull_request:
    branches: [ main ]

jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout repository
        uses: actions/checkout@v4

      - name: Set up JDK 21
        uses: actions/setup-java@v4
        with:
          java-version: '21'
          distribution: 'temurin'
          cache: 'maven'

      - name: Build with Maven
        run: mvn --batch-mode clean verify

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.

✨ Add: GitHub Actions CI workflow to auto-compile Java on every pull request

2 participants