First take at documented coding standards #1429
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR establishes documented coding standards for the terraform-provider-elasticstack repository to improve AI-assisted code review quality. The standards document provides clear guidelines on project structure, schema definitions, JSON handling, resource implementation, testing, and API client usage, while the updated Copilot instructions reference these standards to ensure consistent enforcement during both code writing and review activities.
Key Changes:
- Added comprehensive CODING_STANDARDS.md document covering Go conventions, project structure, and Terraform provider best practices
- Updated Copilot instructions to reference and enforce the new coding standards
- Refined instructions to distinguish between code writing and code review contexts
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| CODING_STANDARDS.md | New document establishing project-wide coding conventions and best practices for Plugin Framework resources, schema definitions, JSON handling, testing, and API client usage |
| .github/copilot-instructions.md | Updated to reference CODING_STANDARDS.md and clarify instructions for both code writing and code review scenarios |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
nick-benoit
left a comment
nick-benoit
left a comment
There was a problem hiding this comment.
Nice, i'm excited to see how this works. Thanks for pushing this forward.
| - Long, multiline descriptions should be stored in an external markdown file, which is imported via Golang embedding. See `internal/elasticsearch/security/system_user/resource-description.md` for an example location. | ||
| - Use schema validation wherever possible. Only perform validation within create/read/update functions as a last resort. | ||
| - For example, any validation that relies on the actual Elastic Stack components (e.g Elasticsearch version) | ||
| can only be performed during the create/read/update phase. |
There was a problem hiding this comment.
I wonder if we should be explicitly asking co-pilot to generate the OAS and consider this when generating schema definitions?
There was a problem hiding this comment.
I guess we could also consider committing this file? I suppose currently we are ignoring the client oas file. I'm sure this would lead to some annoying merge conflicts etc, but maybe is worth it in order to make it easier for copilot to consider the oas?
There was a problem hiding this comment.
I'm sure this would lead to some annoying merge conflicts etc,
Now Renovate is handling the updates we shouldn't have humans cherry-picking changes, so I think these updates should be pretty clean. I'll add it in.
nick-benoit
left a comment
nick-benoit
left a comment
There was a problem hiding this comment.
Nice. I'm looking forward to see how this works in practice. 🚢
* origin/main: First take at documented coding standards (#1429) chore(deps): update golangci/golangci-lint-action action to v9 (#1431) chore(deps): update golang:1.25.4 docker digest to 6ca9eb0 (#1432) chore(deps): update kibana-openapi-spec digest to 96ffcd7 (#1430) chore(deps): update kibana-openapi-spec digest to a8e3b64 (#1428) chore(deps): update golang docker tag to v1.25.4 (#1423)
The intention here is to try and increase the value we get out of Copilot code review. Overall, the aim is to allow human reviewers to focus attention on higher level concerns, with confidence the AI reviews are enforcing more formulaic coding style/expectations, kind of lint on steroids.