Update CONTRIBUTING.md to mention creating a PR#17397
Conversation
Vale Linting ResultsSummary: 1 warning, 2 suggestions found
|
| File | Line | Rule | Message |
|---|---|---|---|
| docs/extend/_publish_an_integration.md | 55 | Elastic.Latinisms | Latin terms and abbreviations are a common source of confusion. Use 'for example' instead of 'e.g'. |
💡 Suggestions (2)
| File | Line | Rule | Message |
|---|---|---|---|
| CONTRIBUTING.md | 12 | Elastic.Wordiness | Consider using 'all' instead of 'all of '. |
| CONTRIBUTING.md | 12 | Elastic.Wordiness | Consider using 'before' instead of 'prior to'. |
The Vale linter checks documentation changes against the Elastic Docs style guide.
To use Vale locally or report issues, refer to Elastic style guide for Vale.
|
@claude let's review the PR Feedback and comments from the last 20 or so PRs (that have comments) and come up with a list of additional items we need to add to We can have some hyper specific items (always run |
|
Claude finished @strawgate's task in 2m 33s —— View job Analyzing PR Feedback for Documentation Improvements
SummaryBased on reviewing ~30 recent PRs with review comments, I've identified the following common feedback patterns that should be documented in Recommended Additions to the "Publish an Integration" Documentation1. Run Pre-submission Checks (High frequency)elastic-package format # Format YAML files consistently
elastic-package build # Build the package
elastic-package lint # Lint the package
elastic-package check # Run all checks
2. Changelog Requirements (Very high frequency)
- version: "1.2.3"
changes:
- description: Description of change
type: enhancement
link: https://github.com/elastic/integrations/pull/XXXXX3. Update CODEOWNERS (Medium frequency)
4. Documentation Consistency (High frequency)
5. Field Definitions (Medium frequency)
6. Ingest Pipeline Best Practices (Medium frequency)
7. Test Coverage (High frequency)
8. Version Bumping (Medium frequency)
9. YAML/Template Best Practices (Medium frequency)
10. Dashboard and SLO Consistency (Medium frequency)
11. Breaking Changes (General guidance)
Items NOT Recommended (Too specific/one-off)These items appeared only once and are too specific to include:
| Why is Claude responding? | Type Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
|
@claude let's add very concise statements for 1, 2, 3, 7 , 8, and 11. After that please review another 20 or so PRs, maybe from 6 months or a year ago and see if there are additional concise items we should add |
|
Claude finished @strawgate's task in 3m 40s —— View job Adding Pre-submission Guidelines to Documentation
SummaryI've updated Items Added (8 total):
Additional Patterns from Older PRs (Not Added - but notable):From reviewing PRs from March-August 2025, I found these recurring patterns:
These were less universal than the items I added, so I left them out to keep the checklist concise. Why is Claude responding? | Type Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
Adds concise guidelines for: - Running pre-submission checks (elastic-package check/format) - Changelog requirements (link field, types) - CODEOWNERS updates for new integrations - Test coverage (elastic-package test, sample_event.json) - Version bumping guidance - Breaking changes documentation - Ingest pipeline error handling - PR title and description best practices Co-Authored-By: Bill Easton <strawgate@users.noreply.github.com>
|
@claude let's review older PRs for things that were flagged as breaking changes and let's include a list of things that are clearly breaking changes (like your |
|
Claude finished @strawgate's task in 3m 2s —— View job Researching Breaking Changes in PR History
SummaryI analyzed 50+ PRs and changelog entries across the integrations repository to identify patterns of breaking changes. I've updated Breaking Changes Added to DocumentationBased on real examples from PR reviews and changelogs:
Why is Claude responding? | Type Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
Expands documentation to include specific types of changes that require a major version bump and breaking-change changelog type: - Field type changes (mapping conflicts) - Field removal/renaming - ECS field collisions - Event value changes - Configuration changes - Data stream restructuring - Transform destination changes - Default behavior changes Co-authored-by: Bill Easton <strawgate@users.noreply.github.com>
|
@claude, if there are breaking changes in the integration, it is possible that dashboards, rules and other content in this repo (and maybe others) would be broken. Let's add some guidance to minimize that risk. |
|
Claude finished @norrietaylor's task in 1m 49s —— View job Adding Guidance for Minimizing Breaking Change Risk
SummaryAdded a new "Minimizing risk from breaking changes" section to the publish documentation. The guidance includes:
Changes pushed in commit 52bce77. Why is Claude responding? | Type Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
Add a new section under 'Document breaking changes' with guidance on how to minimize risk when introducing breaking changes: - Search for dependent content in the repository - Check security_detection_engine package for affected rules - Coordinate with other teams - Consider deprecation periods - Update dependent assets in the same PR Co-authored-by: Norrie Taylor <norrietaylor@users.noreply.github.com>
There was a problem hiding this comment.
The expanded documentation provides helpful guidance for contributors, but needs corrections to align with existing project conventions.
Why is Claude responding? | Type @claude to interact further
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
|
@claude please address |
|
Claude finished @strawgate's task in 45s —— View job Addressing Review Feedback
SummaryI've addressed both review comments:
Changes pushed in commit 18178fd. Why is Claude responding? | Type Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
- Fix changelog type: bug-fix → bugfix (no hyphen) - Remove 'deprecation' type (not used in existing changelogs) - Fix versioning guidance: patch for bug fixes and doc-only changes, minor for new features Co-authored-by: Bill Easton <strawgate@users.noreply.github.com>
🔍 Preview links for changed docs |
🚀 Benchmarks reportPackage
|
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
director |
40000 | 23809.52 | -16190.48 (-40.48%) | 💔 |
To see the full report comment with /test benchmark fullreport
|
@claude please address feedback and do one last review to make sure we didnt miss anything! |
|
Claude finished @strawgate's task in 1m 41s —— View job Addressing Feedback and Final Review
SummaryI've addressed the remaining feedback and completed a final review of the documentation changes. Feedback Addressed
Final Review VerificationI verified the documentation accuracy against the actual codebase:
Changes pushed in commit 2d0dbf7. --- | Why is Claude responding? | Type Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
Update the CODEOWNERS guidance to use the correct absolute path format (/packages/<package_name>) to match the actual file conventions. Co-authored-by: Bill Easton <strawgate@users.noreply.github.com>
There was a problem hiding this comment.
The previous review concerns about changelog types and versioning guidance have been addressed. The one remaining issue (missing leading slash in CODEOWNERS path at line 18) is still unresolved. Once that low-severity fix is applied, this PR is ready to merge.
Why is Claude responding? | Type @claude to interact further
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
axw
left a comment
There was a problem hiding this comment.
Looks good, just one query about changing values for well-defined fields. Changing from a non-compliant value to a compliant one seems like it should be considered a bug fix, though I guess you could argue it's like one of those load-bearing bugs.
| - **Field removal**: Removing fields that users may depend on in dashboards, alerts, or queries | ||
| - **Field renaming**: Renaming fields breaks existing references (dashboards, saved queries, detection rules) | ||
| - **ECS field collisions**: Removing non-ECS fields from ECS namespaces or changing ECS field mappings | ||
| - **Event value changes**: Changing standardized values (e.g., `event.outcome` from `"Succeeded"` to `"success"`) |
There was a problem hiding this comment.
This wouldn't be considered a bug fix?
There was a problem hiding this comment.
A breaking bug fix 😂
💚 Build Succeeded
History
|
Co-authored-by: Bill Easton <strawgate@users.noreply.github.com> Co-authored-by: Norrie Taylor <norrietaylor@users.noreply.github.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
No description provided.