-
Notifications
You must be signed in to change notification settings - Fork 38
Update nuget packages #25
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
base: master
Are you sure you want to change the base?
Update nuget packages #25
Conversation
Thanks for the PR, first check looks very good. |
operation.Parameters[arg.Name] = parameter; | ||
} | ||
|
||
var returnType = |
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.
Check: This must be a proper contextual type to carry NRT info
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 fixed one bug here, is this what you were thinking of? 793d919
var hubModel = new HubModel(hub.Key, hub.Value, resolver); | ||
var template = _settings.TypeScriptGeneratorSettings.TemplateFactory.CreateTemplate("TypeScript", "Hub", hubModel); | ||
artifacts.Add(new CodeArtifact(hubModel.Name, CodeArtifactType.Class, CodeArtifactLanguage.TypeScript, template)); | ||
artifacts.Add(new CodeArtifact(hubModel.Name, CodeArtifactType.Class, CodeArtifactLanguage.TypeScript, CodeArtifactCategory.Undefined, template)); |
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.
Do we need a new category?
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 think Client should be good for this, I fixed this.
Looks good. Ill check the open comments |
Any updates on this? |
Ref: #2 (comment) Cant we use .NET Core 3.0 for more compatibility? |
@TomSmith27 are there things here we still need to merge? |
Looks like this updates to .NET core 3 which could be useful to do however there are conflicts because of the updates i made |
Do you still want to merge this PR? |
I have updated the nuget packages to the latest versions.
NJsonSchema had some interface changes, I'm not sure that I have replaced everything correctly.
I also updated the SignalR version in the Core project, the newer version is part of the aspnet core framework, which can't be referenced in a netstandard project, that's why I changed the targetframeworks. (I couldn't find a workaround to reference the newer version of SignalR without this change.)
The output of the new version is the same, except for this part (
undefined
):But according to the C# classes this seems OK to me.