-
Notifications
You must be signed in to change notification settings - Fork 32
Synonyms as code #2088
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
Synonyms as code #2088
Conversation
# Conflicts: # src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.cs
config/synonyms.yml
Outdated
| @@ -0,0 +1,24 @@ | |||
| synonyms: | |||
| - ".net,.NET,c#,csharp,dotnet,net,NET,DOTNET" | |||
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'm a bit torn on this.
I understand that it was easier to use , as a delimiter instead of having a 2d list here.
I'm a bit worried that this will be prone to undetected errors.
What are your thoughts on this?
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.
Hmm... If it has a parsing error, it would be loud.
But in general, I don't have a strong feeling one way or the other. I imagined this file won't be manipulated nearly as much outside from people involved in looking at the dashboards.
It would be better for this branch's CI to create a quick PR with just the file, so maybe I could update it already.
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.
SGTM. your call
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.
Using commas currently also mimics how synonyms are stored historically in synonym text files. I kinda like it.
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.
sees comment just after changing it
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 mini nitpick
src/tooling/Elastic.Documentation.Tooling/DocumentationTooling.cs
Outdated
Show resolved
Hide resolved
# Conflicts: # config/synonyms.yml
🔍 Preview links for changed docs |
This PR introduces
synonyms.ymlto standardize synonym declarations. The values within are parsed and sent toElasticsearchMarkdownExporter, which performs a PUT request to_synonyms/{setName}.