Conversation
server/Makefile
Outdated
| @@ -0,0 +1,28 @@ | |||
| APP_NAME := myapp | |||
There was a problem hiding this comment.
Since this is a Go backend, consider removing the Makefile for now. Without a deployment target decided, it’s simpler to build directly with Go. Once hosting is finalized, we can add a CD script for build and deployment.
There was a problem hiding this comment.
FYI A Makefile is generally more useful when:
- You need to build for multiple platforms or architectures.
- You want to orchestrate multiple build/test/lint steps.
- You're managing CI/CD pipelines with reusable scripts.
- You're dealing with native C/C++ dependencies or multi-language projects.
server/internal/api/router.go
Outdated
| if _, err := w.Write([]byte("ok")); err != nil { | ||
| log.Printf("failed to write response: %v", err) | ||
| } |
There was a problem hiding this comment.
Remove the error check on w.Write — it’s unlikely to fail for a simple "ok" and only adds clutter.
Linters may suggest this error check, but here it’s unnecessary since the write is trivial and failure unlikely.
| defer func() { | ||
| if cerr := file.Close(); cerr != nil { | ||
| log.Printf("warning: failed to close file: %v", cerr) | ||
| } | ||
| }() |
There was a problem hiding this comment.
| defer func() { | |
| if cerr := file.Close(); cerr != nil { | |
| log.Printf("warning: failed to close file: %v", cerr) | |
| } | |
| }() | |
| defer file.Close() |
server/internal/llm/openai_client.go
Outdated
| rootDir, err := os.Getwd() | ||
| if err != nil { | ||
| return "", err | ||
| } |
There was a problem hiding this comment.
| rootDir, err := os.Getwd() | |
| if err != nil { | |
| return "", err | |
| } | |
| rootDir, err := os.Getwd() |
| @@ -0,0 +1,58 @@ | |||
| name: CI/CD Pipeline | |||
.github/workflows/ci-cd.yml
Outdated
| - name: Build | ||
| run: go build ./... | ||
| working-directory: server | ||
|
|
||
| - name: Run Make Test | ||
| run: make test | ||
| - name: Test | ||
| run: go test ./... | ||
| working-directory: server | ||
|
|
||
| - name: Run Make Lint | ||
| run: make lint | ||
| - name: Lint | ||
| run: golangci-lint run ./... |
There was a problem hiding this comment.
The logical order should be: lint → test → build.
Why:
- Lint first: Catch obvious syntax/style issues early before running expensive operations.
- Test next: Ensure correctness before compiling for deployment.
- Build last: Only build if the code is clean and verified.
|
Note for Approver: |
|
sure, so i will close the pull request for now, and make the required changes, will later open when test cases are added |
You can mark it as draft or do not close at all. I have added comment for approver |
|
@NalinDalal Could you add some test cases for important logics? |
|
@NalinDalal please look at the issue addressing adding test cases |
|
busy with exams🥶 will do surely @peachjelly13 @satyajitnayk |
📝 Description
introduction of ci/cd pipeline so that manual checks such as code compilation can be skipped by automating them
Fixes #16
🔧 Type of Change
🚀 Changes Made
✅ Checklist