Conversation
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive developer documentation to help contributors set up their development environment and understand the development workflow. The documentation addresses issue #770 by providing detailed guidance for new developers.
Key changes:
- Comprehensive developer guide covering setup, workflows, and best practices
- Complete Makefile for standardized development tasks
- Integration of developer documentation into the website navigation
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| website/docs/developers.md | New comprehensive developer guide with setup instructions, workflows, and best practices |
| website/sidebars.ts | Adds "Contributing" section to navigation with link to developer guide |
| Makefile | New comprehensive Makefile with standardized development, testing, and build targets |
| CONTRIBUTING.md | Updates main contributing guide to reference the new developer documentation |
cpuguy83
left a comment
cpuguy83
left a comment
There was a problem hiding this comment.
I think a doc that gets more in the nitty-gritty details of how to run unit tests vs integration tests, where these live, how the integration framework works, etc would be more beneficial here?
| @@ -0,0 +1,206 @@ | |||
| .PHONY: help | |||
There was a problem hiding this comment.
I am generally not a fan of Makefiles like this.
Its not adding anything over go generate ./... and go test.
We do have a docker-bake.hcl which can be useful for executing linters and certain tests and is generally very helpful for lazily building the frontend and feeding that into a build of a spec file without having to save the frontend to an image.
There was a problem hiding this comment.
i think mainly it adds discoverability. users don't really know that they need to run go generate until their PR fails the check
There was a problem hiding this comment.
Would this doc be how this stuff is discovered?
- Update required Go version to 1.24 - Remove unnecessary Makefile targets (build-redirectio, build-all-tools, test-short, test-verbose, version, mod-tidy, mod-verify) - Add TEST_FLAGS variable for flexibility instead of separate verbose target - Consolidate test-integration to handle both all tests and specific suites - Change docs-serve to use 'go -C ./website run .' instead of npm - Add comprehensive test structure documentation explaining unit vs integration tests - Document integration test framework and test file organization - Fix markdown linting issues Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Makefile
Outdated
|
|
||
| .PHONY: generate | ||
| generate: ## Generate required source files | ||
| $(GO) generate ./.../cmd/lin |
There was a problem hiding this comment.
| $(GO) generate ./.../cmd/lin | |
| $(GO) generate ./... |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes #770
Special notes for your reviewer: