Skip to content

Conversation

edkilday
Copy link
Collaborator

@edkilday edkilday commented Sep 27, 2023

Format go files using gofmt
Reintroduce case insensitive method and property names in the javascript bindings
Use method delarations instead of arrow function in the generated typescript
Support optionalclass return values in the javascript bindings
Don't prepend c++ error messages with "Error: "

@edkilday edkilday changed the title A combination of a number of PRs including formatting A combination of a number of commits to tidy up the PR backlog Sep 27, 2023
returnValue := ""
if param.ParamType == "class" {
returnValue = fmt.Sprintf("createV8Instance(objectCreator, param%s, %s_MapClassIdToInjectionClassType(param%s->%s()))", param.ParamName, subComponent.NameSpace, param.ParamName, subComponent.Global.ClassTypeIdMethod)
returnValue = fmt.Sprintf("param%s?createV8Instance(objectCreator, param%s, %s_MapClassIdToInjectionClassType(param%s->%s())):v8::Local<v8::Object>()", param.ParamName, param.ParamName, subComponent.NameSpace, param.ParamName, subComponent.Global.ClassTypeIdMethod) // Very bad
Copy link
Member

Choose a reason for hiding this comment

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

yes, this is a huge mouth full and hard to read. but you are not introducing that pattern here.
I would even go as far as to remove the comment, or atleast say in the comment that you are not happy with the long ternary statement itself.
Would need some refactoring to get rid of...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I left that in accidentally because I did something 'very bad', changed it and then left the comment. I'll get it removed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No its that line. It is just hard to read. I'll do something about that.

@martinweismann martinweismann self-requested a review September 27, 2023 14:58
Copy link
Member

@martinweismann martinweismann left a comment

Choose a reason for hiding this comment

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

please look into my request about the // very bad comment
other than that, looks good!

" retVal = createV8Instance(objectCreator, param%s, %s_MapClassIdToInjectionClassType(param%s->%s()));",
param.ParamName, subComponent.NameSpace, param.ParamName, subComponent.Global.ClassTypeIdMethod)
writer.Writeln("}")
} else if param.ParamType == "enum" {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ahh. I already changed it.

@edkilday edkilday merged commit 343f52f into Autodesk:JSImpl Sep 27, 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.

2 participants