docs: add contributing instructions (#127)#142
Conversation
## **Benefits** 1. **Clarity**: New contributors understand the workflow immediately 2. **Consistency**: Team follows the same practices 3. **AI-friendly**: Claude, Copilot, and other tools can learn project conventions 4. **Safety**: Clear rules prevent accidental breaking changes 5. **Flexibility**: Supports both careful testing and fast iteration --- ## Notes Refer to `CONTRIBUTING.md` for context. I've taken the liberty to make some assumptions / slight tweaks, explained below: 1. Proposed a simplified branch naming convention, using only prefixes `feat/*` and `fix/*`, but we enforce semantic classification (e.g. docs, chore, feat, fix, ci) in PR titles. [[1]](https://github.com/PaperDebugger/paperdebugger/pull/127/changes#diff-eca12c0a30e25b4b46522ebf89465a03ba72a03f540796c979137931d8f92055R95-R96) 2. Note the standard branching flow (proposed to branch off `staging` before raising PR to `staging` and subsequently from `staging` to `main`) [[2]](https://github.com/PaperDebugger/paperdebugger/pull/127/changes#diff-eca12c0a30e25b4b46522ebf89465a03ba72a03f540796c979137931d8f92055R95-R96) - Not sure if we should do this over branching off `main`. On one hand, `main` should always present the latest stable version, but it is also natural to branch off `staging` since it is the target branch of the merge after development. 4. Note the merging strategy [[3]](https://github.com/PaperDebugger/paperdebugger/pull/127/changes#diff-eca12c0a30e25b4b46522ebf89465a03ba72a03f540796c979137931d8f92055R95-R96) **IMPORTANT NOTE**: 1. It seems `make test` is currently failing for some existing test cases. To investigate or inform the relevant owners. 2. @Junyi-99 I have disabled `Automatically request Copilot code review` for PRs to `main` but included it for PRs to `staging` since the new workflow expects heavy / complex changes to first PR to `staging` before `main`. To avoid double-review, i disabled this for PRs to `main`. Let me know if you would like to change or revert this. --- ## **Tasks** - [x] Create `CONTRIBUTING.md` with branching strategy section - [x] Configure branch protection rules in GitHub settings - [x] Set up automated version bumping (if not already configured) - [x] Document this in README.md or link to CONTRIBUTING.md - [ ] Announce new policy to team
…ras3) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract reusable workflows (_build-backend.yml, _build-ext.yml) to eliminate duplicated build/deploy logic across dev/stg/prd. Remove `push: tags: v*` trigger from stg/prd backend workflows to prevent double builds (bump-version already dispatches via repository_dispatch). Delete the no-op build-ext-dev.yml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When mongo.in_cluster is true, PD_MONGO_URI was not set in the configmap, causing the Go app to fall back to localhost:27017 which doesn't work inside the cluster. Now uses the FQDN mongo.<namespace>.svc.cluster.local with the replicaSet parameter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates project contribution documentation and refactors CI/CD by introducing reusable GitHub Actions workflows, while also extending the Helm chart to support node scheduling via node selectors across environments.
Changes:
- Add/augment contribution docs (new
CONTRIBUTING.md, link fromREADME.md). - Refactor backend + extension build/deploy workflows into reusable workflows (
_build-backend.yml,_build-ext.yml) and simplify env-specific workflow files. - Add Helm
nodeSelectorsupport (pluscloudflareNodeSelector) and set env overrides inhack/values-{dev,stg,prd}.yaml.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| helm-chart/values.yaml | Adds top-level nodeSelector and cloudflareNodeSelector values. |
| helm-chart/templates/paperdebugger.yaml | Applies nodeSelector to the main backend Deployment; adjusts in-cluster Mongo URI templating. |
| helm-chart/templates/paperdebugger-xtramcp-server.yaml | Applies nodeSelector to the XtraMCP Deployment. |
| helm-chart/templates/paperdebugger-mcp.yaml | Applies nodeSelector to the MCP server Deployment. |
| helm-chart/templates/mongo.yaml | Applies nodeSelector to the in-cluster Mongo Deployment. |
| helm-chart/templates/cloudflare.yaml | Applies cloudflareNodeSelector to the cloudflared Deployment. |
| hack/values-dev.yaml | Sets dev node selectors for workloads and cloudflared. |
| hack/values-stg.yaml | Sets stg node selectors for workloads and cloudflared. |
| hack/values-prd.yaml | Sets prd node selectors for workloads and cloudflared. |
| README.md | Adds a “Contributing” section linking to CONTRIBUTING.md. |
| CONTRIBUTING.md | Introduces contributor guidelines: branching, protection, PR conventions, and quality checks. |
| .github/workflows/build-ext-stg.yml | Switches to reusable extension workflow. |
| .github/workflows/build-ext-prd.yml | Switches to reusable extension workflow. |
| .github/workflows/build-ext-dev.yml | Deletes the no-op dev extension workflow. |
| .github/workflows/build-backend-dev.yml | Switches to reusable backend workflow. |
| .github/workflows/build-backend-stg.yml | Switches to reusable backend workflow and removes tag push trigger. |
| .github/workflows/build-backend-prd.yml | Switches to reusable backend workflow and removes tag push trigger. |
| .github/workflows/_build-ext.yml | Adds reusable workflow for building + publishing Chrome extension. |
| .github/workflows/_build-backend.yml | Adds reusable workflow for building + deploying backend across dev/stg/prd. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Contributing | ||
|
|
||
| Please refer to [CONTRIBUTING.md](CONTRIBUTING.md) for branch structure, merge workflow, PR guidelines, and code quality requirements. | ||
|
|
There was a problem hiding this comment.
The PR title/description focus on contributing/docs, but this PR also changes Helm scheduling and refactors multiple deploy/build workflows. Please align the PR title/description with the actual scope (or split into separate PRs) to make review/audit history accurate.
| OPENAI_BASE_URL: ${{ inputs.environment == 'dev' && secrets.OPENAI_BASE_URL_DEV || inputs.environment == 'stg' && secrets.OPENAI_BASE_URL_STG || secrets.OPENAI_BASE_URL_PRD }} | ||
| OPENAI_API_KEY: ${{ inputs.environment == 'dev' && secrets.OPENAI_API_KEY_DEV || inputs.environment == 'stg' && secrets.OPENAI_API_KEY_STG || secrets.OPENAI_API_KEY_PRD }} | ||
| INFERENCE_BASE_URL: ${{ inputs.environment == 'dev' && secrets.INFERENCE_BASE_URL_DEV || inputs.environment == 'stg' && secrets.INFERENCE_BASE_URL_STG || secrets.INFERENCE_BASE_URL_PRD }} | ||
| INFERENCE_API_KEY: ${{ inputs.environment == 'dev' && secrets.INFERENCE_API_KEY_DEV || inputs.environment == 'stg' && secrets.INFERENCE_API_KEY_STG || secrets.INFERENCE_API_KEY_PRD }} |
There was a problem hiding this comment.
The secret selection expressions default to the PRD secret when inputs.environment is anything other than dev/stg (e.g., typo/misuse of the reusable workflow). Consider adding an explicit validation step that fails fast for unsupported values, or refactor to use GitHub Environments so the workflow cannot accidentally pick production secrets.
|
|
||
| - name: Build | ||
| run: | | ||
| export GRAFANA_API_KEY=${{ secrets.GRAFANA_API_KEY }} | ||
| cd webapp/_webapp | ||
| npm install |
There was a problem hiding this comment.
Since this workflow uses a committed package-lock.json, prefer npm ci over npm install for deterministic installs in CI. This also typically speeds up builds and avoids lockfile drift; consider enabling setup-node npm caching as well.
| - name: Build | |
| run: | | |
| export GRAFANA_API_KEY=${{ secrets.GRAFANA_API_KEY }} | |
| cd webapp/_webapp | |
| npm install | |
| cache: 'npm' | |
| cache-dependency-path: webapp/_webapp/package-lock.json | |
| - name: Build | |
| run: | | |
| export GRAFANA_API_KEY=${{ secrets.GRAFANA_API_KEY }} | |
| cd webapp/_webapp | |
| npm ci |
| zip -r dist.zip dist/* | ||
|
|
||
| - name: Upload to Chrome Web Store (upload only) | ||
| uses: mobilefirstllc/cws-publish@latest |
There was a problem hiding this comment.
Using mobilefirstllc/cws-publish@latest is a supply-chain risk because it can change without notice. Pin this action to a specific version tag or (preferably) a commit SHA to make builds reproducible and reduce the risk of unexpected behavior.
| uses: mobilefirstllc/cws-publish@latest | |
| uses: mobilefirstllc/cws-publish@v1.5.2 |
Extract reusable workflows (_build-backend.yml, _build-ext.yml) to
eliminate duplicated build/deploy logic across dev/stg/prd. Remove
push: tags: v*trigger from stg/prd backend workflows to preventdouble builds (bump-version already dispatches via repository_dispatch).
Delete the no-op build-ext-dev.yml.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com## Benefits
conventions
Notes
Refer to
CONTRIBUTING.mdfor context. I've taken the liberty to makesome assumptions / slight tweaks, explained below:
feat/*andfix/*, but we enforce semantic classification (e.g. docs,chore, feat, fix, ci) in PR titles.
[1]
stagingbefore raising PR to
stagingand subsequently fromstagingtomain)[2]
main. On one hand,mainshould always present the latest stable version, but it is alsonatural to branch off
stagingsince it is the target branch of themerge after development.
[3]
IMPORTANT NOTE:
make testis currently failing for some existing testcases. To investigate or inform the relevant owners.
Automatically request Copilot code reviewfor PRs to
mainbut included it for PRs tostagingsince the newworkflow expects heavy / complex changes to first PR to
stagingbeforemain. To avoid double-review, i disabled this for PRs tomain. Letme know if you would like to change or revert this.
Tasks
CONTRIBUTING.mdwith branching strategy section