-
Notifications
You must be signed in to change notification settings - Fork 16
Add products
frontmatter
#1226
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
Add products
frontmatter
#1226
Conversation
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.
Some nits but overall LGTM!
return displayAttr?.Name ?? product.ToString(); | ||
} | ||
|
||
public static string? GetEnumMemberValue(this Product product) |
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 depend on a source generator package that allows us to do this without reflection :)
Using that we can also implement the writeyaml method as a oneliner. That way we can roundtrip serialize yamlfrontmatter.
Reviewing on my phone, let me know if I need to provide more targeted pointers!
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.
.Select(value => value!) | ||
.ToList(); | ||
|
||
public static string Suggestion(string input) => |
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.
Love this, all good compilers have this :)
Maybe add it as a more general extension method in Elastic.Documentation? One taking a list of candidates.
Again using the enim source generator package that list can be queried without using reflection.
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.
https://github.com/andrewlock/NetEscapades.EnumGenerators is the alluded package. |
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.
Pull Request Overview
This PR adds support for a new frontmatter property, products, which holds a list of product IDs. Key changes include new tests for products frontmatter, updates to YAML parsing and serialization with a new ProductConverter, and modifications to the layout and documentation files to render and document the products metadata.
Reviewed Changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/Elastic.Markdown.Tests/FrontMatter/YamlFrontMatterTests.cs | Adds tests for single/multiple products and suggestions for spelling/casing errors. |
src/Elastic.Markdown/Suggestions/Suggestions.cs | Introduces a new Suggestion class using a primary constructor for product name suggestions. |
src/Elastic.Markdown/Slices/_ViewModels.cs, Layout/_Head.cshtml, Index.cshtml | Updates view models and templates to include products metadata. |
src/Elastic.Markdown/Myst/YamlSerialization.cs, FrontMatter/Products.cs, FrontMatter/FrontMatterParser.cs | Adds new YAML type converter and products enum definitions to support products frontmatter. |
src/Elastic.Markdown/IO/MarkdownFile.cs | Adds error handling for invalid product values during YAML frontmatter parsing. |
docs/syntax/frontmatter.md | Updates documentation to include the new products frontmatter property. |
Files not reviewed (2)
- Directory.Packages.props: Language not supported
- src/Elastic.Markdown/Elastic.Markdown.csproj: Language not supported
Co-authored-by: Copilot <[email protected]>
Directory.Packages.props
Outdated
@@ -15,7 +15,8 @@ | |||
<PackageVersion Include="Amazon.Lambda.SQSEvents" Version="2.2.0" /> | |||
<PackageVersion Include="AWSSDK.Core" Version="4.0.0.2" /> | |||
<PackageVersion Include="AWSSDK.SQS" Version="4.0.0.1" /> | |||
<PackageVersion Include="AWSSDK.S3" Version="4.0.0.1"/> | |||
<PackageVersion Include="AWSSDK.S3" Version="4.0.0.1" /> | |||
<PackageVersion Include="Supernova.Enum.Generators" Version="1.0.17" /> |
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.
This ItemGroup
is for AWS
packages.
Could we remove NetEscapades.EnumGenerators
all together in favor of Supernova.Enum.Generators
or should we make that a follow up PR?
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.
This ItemGroup is for AWS packages.
Woops, installed it through the UI. 732899d
Could we remove NetEscapades.EnumGenerators all together in favor of Supernova.Enum.Generators or should we make that a follow up PR?
I'd rather do it in a follow-up. Which I can do right after this is merged.
Part of #1200
Changes
Add
products
frontmatter. A list of product IDs.Example
You can find the full list of available product IDs at https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/1226/syntax/frontmatter#products