-
Notifications
You must be signed in to change notification settings - Fork 33
Add docs-builder release-notes create command #2298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow. thank you for the contribution.
src/services/Elastic.Documentation.Services/ChangelogService.cs
Outdated
Show resolved
Hide resolved
|
Another thing that would be nice.. but probably as a follow-up is somehthing like an interactive form. If you don't provide all the args, the CLI asks you. Similar to (gh cli) |
| /// </summary> | ||
| /// <param name="title">Required: A short, user-facing title (max 80 characters)</param> | ||
| /// <param name="type">Required: Type of change (feature, enhancement, bug-fix, breaking-change, etc.)</param> | ||
| /// <param name="products">Required: Products affected in format "product target lifecycle, ..." (e.g., "elasticsearch 9.2.0 ga, cloud-serverless 2025-08-05")</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if applies_to could be the base for it.
Not sure if we can align the syntax. (Also applies_to must be extended to also support dates)
I guess we cannot do this now.. but aligning this in the future could be nice.
I definitely see a connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ I don't think we need to put dates any longer.
Since in our proposal we would discover what serverless release these go out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we were working on the schema, we originally based it on this list of products: https://docs-v3-preview.elastic.dev/elastic/docs-builder/tree/main/syntax/frontmatter#products However, there are some missing from that list (e.g. all the individual EDOT collectors and Elasticsearch Clients, which each have their own set of release notes and thus IMO need a unique product identifier).
Note that since this review, I've edited the PR to handle validation better. Now we can have a list of valid values (for things like the type, subtype, products defined globally in the tool, but individual teams can override that in their changelog.yml config file (if only a subset of the products are applicable to their repo, for example, or they want to allow only a specific list of "areas"). I've also changed it such that if there's a list of valid values and the command diverges from that, the changelog is not generated. Probably additional levels of validation are required (e.g. to ensure that teams can't make up new products, types, or subtypes in their config.yml that don't exist globally) but this is at least a start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ I don't think we need to put dates any longer. Since in our proposal we would discover what serverless release these go out.
I've left that item in the schema for now since at the moment we don't have the longer-term solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make https://github.com/elastic/docs-builder/blob/main/config/products.yml the sole source of truth for allowed products.
We should discuss if all of them are allowed in applies_to, I would argue yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To capture the slack convo:
Florent:
All of what is listed in https://github.com/elastic/docs-builder/blob/main/config/products.yml is not necessarily used in applies_to, some are used exclusively in products or for subs - I don't think it is an issue to extend the list and also use it for release notes. I wouldn't get rid of anything existing if they're still used for some level of search filtering (as part of the products dimension)
Martijn:
Aye we can always denote to not allow a product in applies_to (or elsewhere), for bookkeeping and data integrity/consistency having one list is just nice
All of this to say: all good 👍 👍 :)
src/services/Elastic.Documentation.Services/ReleaseNotesService.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few changes but overall looks good 😍
It'd be nice if we can follow up with this with an interactive mode like gh pr create.
EDIT: @reakaleek already mentioned that :)
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
… combined' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
… combined' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Mpdreamz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One note on products.yml but nothing blocking, this is a great first start 👍
This PR adds a docs-builder command that generates a changelog YAML file.
It is derived from the "elastic-agent-changelog-tool new a-changeset-filename" workflow in https://github.com/elastic/elastic-agent-changelog-tool
For example:
That command creates a file named
1764714171-fixes-prevent-duplication-of-invalid-index-name-st.yamlthat contains:Everything is currently specified on the command line, with only three required options (
title,type, andproduct):Error handing
You can optionally specify a
changelog.ymlfile (in adocsfolder, or specify a different path with the--configoption). It enables you to override the list of acceptable values in thetype,subtype,products,areas, or product lifecycle values. This is useful, for example, if a repo only generates changelogs for a specific subset of products and only has a specific set of areas that ought to appear in changelogs.If you provide a value that doesn't match the schema, you get an error and the changelog is not created.
For example, if I omit
ES|QLfrom the list of valid areas in thechangelog.yml, I get this error:Next steps
elastic-agent-changelog-tool newand require only a filename (per https://github.com/elastic/elastic-agent-changelog-tool/blob/main/docs/usage.md#adding-a-changelog-fragment). However, we'd want to add validation that the minimal information exists before a changelog is merged.That was the intention behind having a label mapping configuration file.
Generative AI disclosure
Tool(s) and model(s) used: composer-1 agent