Skip to content
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 files via upload #1080

Closed
wants to merge 1 commit into from
Closed

Add files via upload #1080

wants to merge 1 commit into from

Conversation

isekene
Copy link

@isekene isekene commented Nov 17, 2022

added timespan type

added timespan type
@isekene
Copy link
Author

isekene commented Nov 17, 2022

@microsoft-github-policy-service agree

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isekene thanks for the contribution. It seems that the files you added were not uploaded at the right place (under their original directory).
Could you move them to the right directory please?

@darrelmiller
Copy link
Member

There is no agreed upon format name for timespan https://spec.openapis.org/registry/format/index.html. Without at least a registry entry, we should not add support for new formats

@baywet
Copy link
Member

baywet commented Jan 4, 2023

@darrelmiller while this PR still needs some work (files at not at the right location for instance) before it can be merged, Kiota maps string + duration to TimeSpan in dotnet (and equivalent types in other languages).
I'm happy to PR the registry with with additional type mappings we have in kiota and the ones that @MaggieKimani1 added in #1094

@darrelmiller
Copy link
Member

@baywet We need to decide why and when we are going to use these native types. Currently we use them in the default property of OpenApiSchema, but in the future that is going to be out of our hands as it will be implemented by a JsonSchema library. I don't think we should try an convert examples into native types during parsing. Parsing the example into native types requires knowing the complete schema which may be referenced and may live in other files. I think if users want to translate an OpenApiExample into a native object, then we should have separate code to do that. I don't think we need the full set of primitive types.

@baywet
Copy link
Member

baywet commented Jan 13, 2023

@darrelmiller the aspect that this PR tries to parse the examples was lost on me because of the formatting/files being at the wrong place.
Outside of examples, just to represent the schema, unless we decide to take a dependency on a JSON schema lib, which is not the case today, I'd argue that having types that represent known type/format combinations is actually a good thing.
People won't have to remember the exact names/combinations, they don't risk making typos, generators would be able to use type inference instead of string comparison, etc...
Thoughts?

@darrelmiller
Copy link
Member

@baywet The problem we have is that even today, validation of examples will fail if a schema is a $ref. That's one reason I changed the example validation to be a warning, not an error. Also, we run into issues like this #1106 where we try and represent a JSON number but there is no equivalent type in .NET.

@baywet
Copy link
Member

baywet commented Oct 24, 2023

closing due to inactivity from author

@baywet baywet closed this Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants