docs: enforce code signoff in AGENTS.md #705
Conversation
Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
- Version: Changed from 5.0.0 to 4.2.0 per reviewer request - Added experimental/unstable warning to README and NOTES.txt - Removed cassandra-schema.yaml (Jaeger v2 handles schema internally) - Removed schema: section from values.yaml - Refactored storage-configmap.yaml to use blob-style config - Updated README with native Jaeger/OTEL config examples - Clarified Cassandra support status in documentation Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
Signed-off-by: Jonah Kowall <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR adds a DCO (Developer Certificate of Origin) signoff requirement to the AGENTS.md file to enforce code signing for all commits. However, the PR includes extensive changes beyond documentation, including a major refactor of the Jaeger Helm chart to version 4.2.0 with architectural simplifications, template consolidations, and configuration updates.
Key changes:
- Added DCO signoff requirement in AGENTS.md
- Consolidated Elasticsearch maintenance jobs into a single template file
- Unified Jaeger services into a single Service manifest
- Removed deprecated components (hotrod, cassandra-schema)
- Simplified storage configuration approach
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| AGENTS.md | Added DCO signoff requirement in "Always do" section |
| charts/jaeger/values.yaml | Removed deprecated storage config, simplified YAML formatting, removed hotrod and schema sections |
| charts/jaeger/templates/es-maintenance.yaml | New consolidated file merging esIndexCleaner, esRollover, and esLookback CronJobs with ServiceAccounts |
| charts/jaeger/templates/service.yaml | New unified Service replacing separate agent/collector/query services |
| charts/jaeger/templates/storage-configmap.yaml | Refactored to use helper templates for configuration sections |
| charts/jaeger/templates/spark-dependencies.yaml | Added conditionals for volumeMounts/volumes, moved ServiceAccount inline, updated Elasticsearch config approach |
| charts/jaeger/templates/_helpers.tpl | Removed deprecated helpers, added new elasticsearch.env and extra.cmdArgs helpers |
| charts/jaeger/templates/*.yaml (deleted) | Removed 13 separate template files for deprecated components |
| charts/jaeger/README.md | Complete rewrite with v4.2.0 architecture documentation |
| charts/jaeger/Chart.yaml | Bumped version to 4.2.0 |
| Makefile | Adjusted Elasticsearch readiness probe timeouts |
| .vscode/settings.json | Added VS Code configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| imagePullPolicy: {{ .Values.esRollover.image.pullPolicy }} | ||
| args: | ||
| - rollover | ||
| - rollover |
There was a problem hiding this comment.
The args array contains a duplicate value "rollover" on lines 182-183. The first argument should be the command name, and the second should likely be the Elasticsearch URL or another parameter, not a duplicate of the command.
| - rollover |
| imagePullPolicy: {{ .Values.esLookback.image.pullPolicy }} | ||
| args: | ||
| - lookback | ||
| - lookback |
There was a problem hiding this comment.
The args array contains a duplicate value "lookback" on lines 302-303. The first argument should be the command name, and the second should likely be the Elasticsearch URL or another parameter, not a duplicate of the command.
| - lookback |
| imagePullPolicy: {{ .Values.esIndexCleaner.image.pullPolicy }} | ||
| args: | ||
| - {{ .Values.esIndexCleaner.numberOfDays | quote }} | ||
| - {{ .Values.esIndexCleaner.numberOfDays | quote }} |
There was a problem hiding this comment.
The numberOfDays value is duplicated in the args on lines 51-52. This appears to be a copy-paste error where the same value is passed twice to the index cleaner command.
| - {{ .Values.esIndexCleaner.numberOfDays | quote }} | |
| - {{ .Values.esIndexCleaner.prefix | quote }} |
| {{- if .Values.allInOne.service.annotations }} | ||
| {{- toYaml .Values.allInOne.service.annotations | nindent 4 }} | ||
| {{- end }} |
There was a problem hiding this comment.
The annotations block on lines 8-10 is placed within the labels section. Annotations should be at the same level as labels in the metadata section, not nested inside labels. This would result in invalid Kubernetes YAML.
| {{- if .Values.allInOne.service.annotations }} | |
| {{- toYaml .Values.allInOne.service.annotations | nindent 4 }} | |
| {{- end }} |
| --set provisionDataStore.cassandra=true \ | ||
| --set storage.type=cassandra | ||
| elasticsearch: | ||
| server_urls: ["http://elasticsearch:9200"] |
There was a problem hiding this comment.
The comment on line 62 states "Or connect to an external Elasticsearch cluster" but the example config shown doesn't demonstrate connecting to an external cluster. The server_urls still points to a generic "http://elasticsearch:9200" which could be confusing. Consider showing a more concrete external example like "http://my-external-es.example.com:9200" to clarify the distinction from the provisioned mode.
| server_urls: ["http://elasticsearch:9200"] | |
| server_urls: ["http://my-external-es.example.com:9200"] |
1 participant
Please verify that the DCO signoff requirement is clear and correctly placed in AGENTS.md.