Extend CONTRIBUTING.md #1262
Extend CONTRIBUTING.md #1262
Conversation
CONTRIBUTING.md
Outdated
| * Add a changelog entry in `CHANGELOG.md` under the `Unreleased` section. This will be included in the release notes of the next release. | ||
| * Submit your PR for review. |
There was a problem hiding this comment.
Currently changelog entries link back to the PR introducing the change which means we first need to open the PR and then add the changelog entry. That situation has never really made sense, but it does make sense to be consistent for now.
IMO rather than improving the ordering here, it would likely make more sense to look at automatically adding changelog entries based on the PR title.
There was a problem hiding this comment.
I'll reorder it for now. Though it would be nice to have this automated so the release job just collects release notes from all PRs included in the release.
CONTRIBUTING.md
Outdated
| * Use an existing resource (e.g. `internal/elasticsearch/security/system_user`) as a template. | ||
| * Some resources use the deprecated Terraform SDK, so only resources using the new Terraform Framework should be used as reference. | ||
| * Use the generated API clients to interact with the Kibana APIs. (See [Working with Generated API Clients](#working-with-generated-api-clients) | ||
| * Add documentation and examples for the resource. Update the generated docs with `make docs-generate`. |
There was a problem hiding this comment.
We also need to add the doc template (example) as part of this work.
The docs generator doesn't (or didn't, I haven't checked recently) populate the subcategory field in the frontmatter which we're using to improve the docs organisation.
CONTRIBUTING.md
Outdated
| ## Using Copilot | ||
|
|
||
| Update documentation templates in `./templates` directory and re-generate docs via: | ||
| ```bash | ||
| make docs-generate | ||
| ``` | ||
| GitHub Copilot can speed up development, but you’re responsible for correctness: | ||
| * Create an issue describing the desired change and acceptance criteria. | ||
| * Assign the issue to Copilot and iterate with prompts. | ||
| * Review outputs carefully; add tests and adjust as needed. | ||
| * Example issue: https://github.com/elastic/terraform-provider-elasticstack/issues/1219 |
There was a problem hiding this comment.
So this is kind of correct, but with some limitations.
- Contributions should already have an open issue, people shouldn't be creating an issue just to get coding agent involved. We also want to create some guidance about the content of that issue, we should aim to have some example issues which have provided good prompts to coding agent.
- There's restrictions on who can assign an issue to Copilot. We need to make sure folks have a clear path there
- IMO this section is targeted purely at Elasticians, whilst the rest of this doc mostly makes sense for any community members. I think there's enough content here (i.e 'Adding support for Stack features via Copilot') to have a dedicated internal doc.
There was a problem hiding this comment.
I'll remove it from this doc for now.
Should we have a separate doc in the repo for internal docs? The release process is also something that only applies to Elasticians.
CONTRIBUTING.md
Outdated
| * `make fmt`: Format the code. | ||
| * `make lint`: Lint the code. |
There was a problem hiding this comment.
| * `make fmt`: Format the code. | |
| * `make lint`: Lint the code. | |
| * `make lint`: Lint the code. |
Lint also formats
CONTRIBUTING.md
Outdated
|
|
||
| List of previous commits is a good example of what should be included in the changelog. | ||
| * `make build`: Build the provider. | ||
| * `make install`: Install the provider. |
There was a problem hiding this comment.
I don't know why we maintain this make target tbh, I've never used it and would suggest that using it as part of a development workflow is broken. IMO we shouldn't mention this as part of a contributing guide.
Should we remove the make target? It depends, I guess it makes sense if any users are building the provider from source. Do we need to make that workflow seamless though...
There was a problem hiding this comment.
Yeah I also never use it. The default workflow should probably be to use the debugging env-var. I'll remove it from the guide.
CONTRIBUTING.md
Outdated
| ```bash | ||
| make fmt | ||
| make docker-testacc |
There was a problem hiding this comment.
This path is can be quite slow, it both spins up the Elastic Stack docker containers (Yay!), but also runs the actual provider tests inside a golang docker container.
Repeated testing (i.e what we'd expect in a dev workflow) is much faster if we re-use a single running Stack environment but also run the tests directly on the dev machine.
| make docker-testacc | |
| make docker-fleet | |
| make testacc |
There was a problem hiding this comment.
Sounds good, I updated the docs to use this faster workflow.
CONTRIBUTING.md
Outdated
| - `generated/alerting` | ||
| - `generated/connectors` | ||
| - `generated/slo` | ||
| - `internal/clients/*`: Manually written clients. These should only be used/extended if it is not possible to use the generated clients. |
There was a problem hiding this comment.
| - `internal/clients/*`: Manually written clients. These should only be used/extended if it is not possible to use the generated clients. |
This isn't really true. Much of this package configures one of the Stack clients, providing endpoints/credentials/etc from the provider configuration to the client.
There's some code in here which is used as a bridge between the provider resources and the older Kibana/Elasticsearch clients. We don't need to continue with that as much, but there are still cases where it may make sense, for instance in Kibana connectors have a bunch of non-trivial logic managing API defaults which kind of makes sense in a client layer.
I don't think there's a need for a hard rule on not making additions here.
There was a problem hiding this comment.
True, I didn't realize this also contains all the wrapper code for the generated client. I'll remove that entry.
gigerdo
force-pushed
the
update-contributing-doc
branch
from
eaa2516 to
f35eec4
Compare
gigerdo
force-pushed
the
update-contributing-doc
branch
from
f35eec4 to
2e25dec
Compare
* origin/main: Migrate Elasticsearch enrich policy resource and data source to Terraform Plugin Framework (#1220) Add support for solution field in elasticsearch_kibana_space resource and data source (#1210) Don't force replacement when changing integration versions (#1255) chore(deps): update golang:1.25.0 docker digest to 5502b0e (#1267) chore(deps): update docker.elastic.co/kibana/kibana docker tag to v9.1.3 (#1263) Bump github.com/ulikunitz/xz from 0.5.12 to 0.5.14 (#1264) Migrate Kibana connectors to use the bundled openapi generated client (#1260) Extend CONTRIBUTING.md (#1262) fix: add `namespace` attribute to `elasticstack_kibana_synthetics_monitor` resource (#1247) Bump github.com/stretchr/testify from 1.10.0 to 1.11.0 (#1261) chore(deps): update codecov/codecov-action digest to fdcc847 (#1258) fix(deps): update module go.uber.org/mock to v0.6.0 (#1259)
Add some more detailed instructions on how to write code for the provider. This is more of a starting point which can be extended further. The goal is to have a basic guidance for anyone new to the provider development: Which plugin to use, which resource to use as a baseline example, etc.