Skip to content

Add CONTRIBUTING.md#352

Open
morissette wants to merge 5 commits intoCodeForPhilly:mainfrom
morissette:main
Open

Add CONTRIBUTING.md#352
morissette wants to merge 5 commits intoCodeForPhilly:mainfrom
morissette:main

Conversation

@morissette
Copy link
Contributor

Summary

Adds a CONTRIBUTING.md to guide new developers onboarding to the project.

What's covered

  • Ways to contribute (code, docs, DMN rules, testing PRs)
  • Project structure overview with the Java 17 vs 21 split called out explicitly
  • Development environment setup for all four options (Codespaces, Devbox, Devcontainer, DIY)
  • Service startup order and dependency chain
  • Making changes (branching, commits, library metadata sync)
  • Testing (Java unit tests, Bruno API tests, Playwright e2e)
  • Code style (Prettier config, DMN naming conventions)
  • PR submission process referencing CODEOWNERS
  • Working with DMN files — step-by-step for adding benefits and checks, with naming constraints
  • Deployment workflow for maintainers

Test plan

  • Review document for accuracy against current project setup
  • Verify all links resolve correctly
  • Confirm DMN naming rules and Java version callouts are accurate
  • Check that deployment steps match current maintainer workflow

🤖 Generated with Claude Code

morissette and others added 2 commits March 10, 2026 21:12
Covers environment setup, project structure, development workflow,
testing, code style, PR process, DMN file conventions, and deployment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@morissette morissette requested a review from prestoncabe as a code owner March 11, 2026 01:34
…G.md

- devbox.json only provides JDK 21; library-api targets Java 17 via
  Maven --release 17, not a separate JDK install
- Add builder-api (port 8081) to Service URLs table
- Clarify DIY setup requires JDK 21 only, not both 17 and 21
Copy link
Contributor Author

@morissette morissette left a comment

Choose a reason for hiding this comment

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

Reviewed this against the source code. Overall the doc is solid — structure, service startup order, script paths, port numbers, Prettier config, CODEOWNERS, deploy triggers, and DMN naming conventions all check out. Found two issues that have been corrected in the latest push:

Bug fix: Java version claim was inaccurate

devbox.json only declares jdk21@latest — there is no JDK 17 in the Nix environment. The original doc said Devbox manages "Java 17, Java 21" which would mislead a contributor trying to understand the setup. In practice this works fine because library-api/pom.xml uses maven.compiler.release=17, so JDK 21 compiles it targeting Java 17 bytecode via --release 17. Updated the Devbox section, DIY section, and the Project Structure callout to reflect this accurately.

Bug fix: builder-api missing from Service URLs table

builder-api runs on port 8081 (confirmed in builder-api/.env) but wasn't in the reference table. Added it.

Everything else verified as accurate:

  • All 5 service ports match firebase.json and process-compose.yml
  • Startup order matches process-compose.yml dependency chain exactly
  • All bin/ script paths exist
  • Bruno test path library-api/test/bdt exists
  • library-api-v* deploy trigger confirmed in .github/workflows/deploy-library-api.yml
  • KIE DMN version 10.0.0 confirmed in builder-api/pom.xml
  • Node 22 confirmed in devbox.json
  • builder-frontend at 5173 confirmed in vite.config.js
  • Firebase emulator project demo-bdt-dev confirmed in .firebaserc

Development environment setup is already covered in detail in README.md.
CONTRIBUTING.md now links there instead of repeating the same content.
Copy link
Collaborator

@prestoncabe prestoncabe left a comment

Choose a reason for hiding this comment

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

Thanks @morissette - I noticed a few small things, but otherwise good.

- Mention .vscode/java.format.xml in the Java style section
- Update DMN section intro wording per reviewer suggestion
- Add tip about /new-dmn-check CC command for new check boilerplate
- Replace manual gcloud sync steps with GH Action trigger recommendation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@morissette morissette requested a review from prestoncabe March 12, 2026 12:26
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.

2 participants