microsoft / Chakra-Samples

Repository for Chakra JavaScript engine related samples.
MIT License
216 stars 84 forks source link

Fixed a typos in C# examples #56

Closed Taritsyn closed 7 years ago

liminzhu commented 7 years ago

Thank you for the PR @Taritsyn ! As someone could be depending on the C# wrapper already, l think only absolutely necessary changes should get in. Looking at the corrections,

TypeInt32 (this was added before I started to work on Chakra so this is my guess) is probably trying to avoid name collision with System.Int32.

JavaScriptPropertyIdTypeString/Symbol actually mirrors the their C++ type names so I'd probably leave this one as is.

Taritsyn commented 7 years ago

Hello, Limin Zhu!

TypeInt32 (this was added before I started to work on Chakra so this is my guess) is probably trying to avoid name collision with System.Int32.

If this were so, then the same would be done for Int16. It is a trivial typo: copied the fragment from the C++ source code, but made a mistake when removing a JsArrayType prefix.

JavaScriptPropertyIdTypeString/Symbol actually mirrors the their C++ type names so I'd probably leave this one as is.

In this case, just forgot to remove the JsPropertyIdType prefix. These typos violate the common style of examples.

As someone could be depending on the C# wrapper already, l think only absolutely necessary changes should get in. Looking at the corrections,

This is open source, moreover, it is examples of usage. Nothing bad will happen, if they change. It will be bad if people will copy the “bad” code in their projects.

liminzhu commented 7 years ago

Gave it more thoughts last night and I think what you said makes sense. Going to merge it.

Thanks again for the PR!