-
Notifications
You must be signed in to change notification settings - Fork 190
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
Do not consider abstract classes in configuration options #362
Conversation
@gsteenpa Thanks for the PR! However, I wonder whether this really needs or not because there's no abstract class in the code implementing |
Hi - We have an abstract base in our usage of the library so that we can share some settings (like setting a standard copyright with a dynamic year, forcing a specific openapi spec version, and ensuring a standard api title/description) across various apis. Each api then inherits from that base class in its implementation (the "concrete" class) to set api specific information. |
Oh, I see. Then, it would make senses. |
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.
Would you please add test cases to validate your change?
...t.Azure.WebJobs.Extensions.OpenApi.Core.Tests/Resolvers/OpenApiConfigurationResolverTests.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.
Thanks for your contribution! It LGTM
We use an abstract base class to share some configuration options. The resolver is picking up that abstract class and thowing an error on the SingleOrDefault.
This is the exception stack: