Skip to content

CI-CD this closes #26#141

Open
lohaniprateek wants to merge 5 commits intoadhit-r:mainfrom
lohaniprateek:main
Open

CI-CD this closes #26#141
lohaniprateek wants to merge 5 commits intoadhit-r:mainfrom
lohaniprateek:main

Conversation

@lohaniprateek
Copy link

@lohaniprateek lohaniprateek commented Nov 23, 2025

📝 Description

A brief description of what this PR does.

🔗 Related Issue

Closes #(issue number)

🎯 Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Performance improvement
  • Test addition or update

🧪 Testing

How has this been tested?

  • Unit tests
  • Integration tests
  • Manual testing
  • E2E tests

Test Steps:

  1. Step 1
  2. Step 2
  3. Step 3

📸 Screenshots (if applicable)

Add screenshots to help explain your changes.

✅ Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

🔍 Review Notes

Any special notes for reviewers:

  • Note 1
  • Note 2

📚 Additional Context

Add any other context about the PR here.


Thank you for contributing to FairMind! 🎉

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces CI/CD infrastructure for both frontend and backend applications, addressing issue #26. It adds GitHub Actions workflows for automated testing, building, and deployment, along with a Dockerfile for the frontend application.

Key changes:

  • Added GitHub Actions workflows for frontend and backend CI/CD pipelines with test, build, and deploy stages
  • Introduced a multi-stage Dockerfile for the Next.js frontend application using Bun
  • Configured automated deployment to Railway with Docker image publishing to GitHub Container Registry

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 16 comments.

File Description
apps/frontend/Dockerfile Multi-stage Dockerfile for building and running the Next.js frontend with Bun runtime
.github/workflows/frontend_ci.yml CI/CD workflow for frontend including linting, testing, Docker build/push, security scanning, and Railway deployment
.github/workflows/backend_ci.yml CI/CD workflow for backend including Python linting, type checking, security scanning, Docker build/push, and Railway deployment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- name: Build and push
uses: docker/build-push-action@v4
with:
context: apps/frontend
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

The Docker build context is set to apps/frontend, but the Dockerfile copies files from apps/frontend/ within the container. This creates a mismatch where the Dockerfile expects to be run from the repository root, but the workflow sets the context to the apps/frontend directory.

This will cause the build to fail because paths like COPY apps/frontend/package.json won't work when the context is already apps/frontend.

Suggested fix: Either change the context to . (repository root):

context: .
file: apps/frontend/Dockerfile

Or update the Dockerfile to work with apps/frontend as the context by removing the apps/frontend/ prefix from COPY commands.

Suggested change
context: apps/frontend
context: .

Copilot uses AI. Check for mistakes.
Comment on lines +4 to +10
on:
push:
branches:
- main
pull_request:
branches:
- main
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

[nitpick] The workflow uses push to main branch as a trigger (lines 5-7), which means every push to main will trigger build, push, and deploy. This could lead to unnecessary deployments if multiple commits are pushed quickly or if the deploy should only happen on releases.

Consider adding conditions to prevent deployment on every push:

on:
  push:
    branches:
      - main
    tags:
      - 'v*'  # Deploy only on version tags
  pull_request:
    branches:
      - main

jobs:
  # ... test and build jobs ...
  
  deploy:
    runs-on: ubuntu-latest
    needs: build_and_push
    if: github.event_name == 'push' && github.ref == 'refs/heads/main'  # Only deploy on push to main, not PRs

Copilot uses AI. Check for mistakes.
@adhit-r
Copy link
Owner

adhit-r commented Nov 28, 2025

@kishore8220 -> pls review

@adhit-r
Copy link
Owner

adhit-r commented Nov 28, 2025

Hey! Thanks for the PR. I found a few issues that need fixing:

Main Issues:

  1. Wrong frontend path - The workflows and Dockerfile reference apps/frontend but the actual frontend is in apps/frontend-new. This will cause the workflows to fail.

  2. Duplicate workflows - We already have CI/CD workflows in .github/workflows/ci-cd.yml that do similar things. This might cause conflicts or duplicate runs.

  3. Missing type-check script - The frontend workflow runs bun run type-check but that script doesn't exist in apps/frontend-new/package.json. The existing workflow uses bunx tsc --noEmit instead.

  4. Backend should use uv - The backend workflow uses pip but we prefer uv for Python (see existing ci-cd.yml).

  5. Outdated action versions - Using actions/checkout@v3 and setup-python@v4 while existing workflows use v4/v5.

  6. Missing permissions - The workflows don't set proper permissions which might cause issues.

  7. Dockerfile path issue - The frontend Dockerfile references apps/frontend which doesn't exist.

Suggestions:

  • Either update the paths to use apps/frontend-new or consolidate with the existing workflows
  • Use uv for backend instead of pip
  • Add proper permissions blocks
  • Update action versions to match existing workflows
  • Fill out the PR description template (it's currently empty)

@lohaniprateek
Copy link
Author

I'll check that.

@kishore8220
Copy link
Collaborator

The CI/CD pipeline implementation is well-structured and covers the necessary stages for build, test, and deployment. I appreciate the use of environment variables and the separation of concerns within the workflow files.
However, it would be beneficial to include more granular error handling, especially for deployment steps, to ensure easier debugging in case of failure. Please also consider adding caching for dependencies in the build stage to improve performance.
Lastly, a brief README update summarizing the workflow changes would help future contributors quickly understand the CI/CD process.

Copy link
Owner

@adhit-r adhit-r left a comment

Choose a reason for hiding this comment

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

Thanks for the effort on this CI/CD implementation. After reviewing the changes, I found that we already have a comprehensive CI/CD pipeline in place at .github/workflows/ci-cd.yml that covers the same functionality.

The current approach creates duplicate workflows which will cause:

  • Multiple redundant CI runs on every push and PR
  • Inconsistent tool usage (pip vs uv for Python)
  • Outdated GitHub Actions versions (v3/v4 instead of v4/v5)
  • Path mismatches (apps/frontend instead of apps/frontend-new)

Instead of adding separate workflow files, I'd recommend enhancing the existing ci-cd.yml with the improvements you're proposing. This approach will:

  • Keep CI/CD configuration in one place
  • Avoid resource duplication
  • Maintain consistency with project conventions

Looking at your implementation, the reviewer (kishore8220) already gave positive feedback on the structure. What we can do is incorporate those good ideas into the existing workflow:

  1. Add dependency caching to improve build performance
  2. Enhance error handling in deployment steps
  3. Add a README section documenting the CI/CD process for future contributors

Would you be able to rework this PR to update ci-cd.yml instead of creating new workflows? This will be more maintainable long-term and align with our current architecture.

Happy to help guide the changes if you have questions about how to structure them.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Prateek Kumar Lohani <151424017+lohaniprateek@users.noreply.github.com>
@safedep
Copy link

safedep bot commented Feb 18, 2026

SafeDep Report Summary

Green Malicious Packages Badge Green Vulnerable Packages Badge Green Risky License Badge

No dependency changes detected. Nothing to scan.

Installation is not linked with SafeDep Tenant. Click here to optionally link your GitHub App installation with SafeDep Tenant.

This report is generated by SafeDep Github App

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.

3 participants