-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Clean up ApiGenerator #3755
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
Clean up ApiGenerator #3755
Conversation
…ds for preferred HTTP methods
now we only yield methods for the preferred HTTP method
endpoint.MethodName = tokens.Last(); | ||
if (tokens.Length > 1) | ||
endpoint.Namespace = tokens[0]; | ||
//todo side effect |
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.
Stray TODO?
MergeArrayHandling = MergeArrayHandling.Union | ||
}); | ||
|
||
if (pathsOverride != null) original.SelectToken("*.url.paths").Replace(pathsOverride); |
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.
👍 simplifies the .replace.json
files...
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.
LGTM, I've left a few small comments.
} | ||
@if (names.DescriptorNotFoundInCodebase) | ||
{<text> | ||
//TODO THIS METHOD IS UNMAPPED! |
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've always wondered if we ought to introduce a deliberate compiler error here instead, to prompt us to do the right thing and fix it.... thoughts?
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 want to generate these into Unmapped.Requests|Response.<MethodName>.cs
where we generate stubs for Request
and Response
with obsoletes with compiler errors on them.
This makes adding a new API easier and makes sure we won't release a stub.
</text> | ||
} | ||
@if (names.DescriptorNotFoundInCodebase) | ||
{<text> //TODO THIS METHOD IS UNMAPPED! Expected to find @names.DescriptorName and @names.RequestName in a file called @(names.RequestName).cs in NEST's codebase |
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.
Introduce a deliberate compiler error here instead?
This PR cleans up the ApiGenerator a tad. One of the oldest parts of the codebase dating back to
1.0
The razor views can now be split using partials and they use more dedicated models for each individual component. In the past everything was driven by
CsharpMethod
now request/interfaces/et all have specific domain models.The generated files are now also automatically indented so we need to fiddle a whole lot less with whitespace in the razor views.
We no longer generate alternative http methods on the low level client so things such as
client.SearchGet()
are a thing of the past.This PR also serves as jumping point for introducing namespaces and the domain model should help massively when we want to reimplement the yaml test runner next.