-
Notifications
You must be signed in to change notification settings - Fork 98
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
Convert to TypeScript? #64
Comments
Pros:
Cons:
Personally I love Typescript, but whether I support it here or not, I don't know yet. |
I would actually downvote this idea (yes my own proposal!) for the following reasons:
I think there are some excellent points by @ericelliott in the following articles:
Thanks @breautek for the quick reaction! |
This was the primarily reason why I wasn't like "YES LETS DO IT!" |
I was the person who contacted @brodybits about converting this to typescript. It appears as if our organization may be requiring a significant number of Cordova apps and modifying the IOS projects in plugins has been a significant issue. I had started creating a .d.ts file for this library only to estimate that doing this correctly was on the order of magnitude similar to converting the code to typescript. So I bit the bullet and did it. I know the community might not appreciate typescript and might not pull this into the current repository. And if so, that is fine. If that is the case, I will probably just maintain this typescript version of the library for our internal uses and remove some of the inconsistencies that were required to allow the current tests to continue to pass and support the current API for code that might have been deployed against this package. If you are interested in seeing what this looks like and hopefully providing some feedback, the code is sitting in the github branch: https://github.com/dball-adashi/cordova-node-xcode/tree/typescript-migration-eval At the present time I have:
At the present time, I have not:
I.e. this is not ready for a pull. I am new to contributing to open source and this is the first significant bit of code that I have ported to Typescript. I usually write from the ground up. So I am happy and hopeful to hear any feedback about this library, version, or anything related to this effort so that I may improve my future efforts. Thank you. |
I can't foresee it being merged into the main repo, but I don't think there is anything wrong maintaining your own version of the library. |
Hi @dball-adashi, that is the beauty of open-source! My one major criticism is that you seemed to have "borrowed" from a GPL licensed project. I think it should be obvious that this would be unacceptable in a PR on an Apache project and bad for permissive licensing in general. In case you do need something from a GPL licensed project, I would recommend you consider moving out into its own component. Otherwise, please feel free to update and publish as you feel would be appropriate. AFAIC link to your fork would be welcome if you can keep the GPL code out. |
Brody, Should I assume at this point that once I complete my version either? The reason I ask is that I removed errors and validation checks I would normally leave in a library to make sure it is being used correctly simply so that it could pass the existing unit tests. I also left improperly (inconsistently) named methods so that it would remain compatible with existing clients. @breautek and @brodybits, thank you for your prompt replies and attention to this. I appreciate every comment and set of information you provide. |
I would say "no", your code is OK. I think it would be good to give credit for the information that you used from the project, and how you just used the information rather than directly copying the code due to this licensing issue. And I think the fact that the other project is in a different language makes this especially clear.
My response coming soon... |
I would not make that assumption 100%. I wold also be reluctant to confirm your other assumption that it "may be pulled back into apache/cordova-node-xcode" at this point. IMHO a fork that only does the port to TypeScript would not help a lot of other people. I would personally favor using JSDoc with tsc (TypeScript compiler) to enforce strong typing instead, as I proposed in #65. But: if you or anyone else does make a fork in TypeScript that includes some other improvements, and with better support, it may be able to help some other users in the community. For example, I noticed that React Native does use this package for its CLI, which seems to be the one major component in TypeScript: https://github.com/react-native-community/cli/tree/master/packages/platform-ios/src |
In our application, I've had to add a hacky TypeScript layer (not a proper typings file) to make using the methods I needed from this library a bit easier. I'd be thrilled to have a proper TypeScript layer. I think TypeScript is awesome for the somewhat self documenting nature of it, when I needed to add some extra features or work around some bugs it was brutally difficult to figure out what attributes were expected/allowed/where I needed to patch things just with the JS/pbxproj output. |
@fbartho I would love to get your feedback. If you replace your Xcode dependency in the package.json file with the following temporarily and disable your manually created .d.ts files from being considered by typescript, you should get a sense of the design. Your code should run unchanged. That is unless the type checking indicates there is a problem. It would still run as the logic is unchanged. "dependencies": { |
For anyone in need of this, this is the best solution so far:
This will generate a xcode.d.ts file in the project root, move it into the src folder.
The final file should look like this:
Everything is declared as any but at least function and parameter names are generated and you can modify types for the functions you need easily. Maybe a hero sets types for all methods and shares with us. |
Someone asked me by email if there would be any objections to converting this project to TypeScript in a pull request.
This kind of a contribution is always welcome for discussion and consideration. It would be best to first raise an issue for discussion.
My personal feelings about this idea are coming in a comment. I will also cast a vote on my own issue, based on my personal feelings coming up.
The text was updated successfully, but these errors were encountered: