telerik / mobile-cli-lib

Contains common infrastructure for CLIs - mainly AppBuilder, NativeScript, DDB and Proton.
Apache License 2.0
11 stars 10 forks source link

fix getxcodeversion usage #996

Closed Plamen5kov closed 7 years ago

Plamen5kov commented 7 years ago

problem related issue: https://github.com/NativeScript/nativescript-cli/issues/2810 getxcodeversion returning null isn't handled correctly in tns doctor

solution handle erroneous case and remove unnecessary code

rosen-vladimirov commented 7 years ago

This code will not fix the mentioned issue, it will just fail with different error when executing tns doctor. By definition tns doctor should detect all issues and print them, it should not fail and stop execution once a single issue is detected.

@Plamen5kov am I missing something here, maybe there's another PR in {N} CLI that would fix the doctor failure?

Plamen5kov commented 7 years ago

@rosen-vladimirov I thought failing was the expected behavior, my bad. I'll fix it in another PR.

rosen-vladimirov commented 7 years ago

@Plamen5kov the failure in the current code (changed in this PR) is fine IMO. But tns doctor should catch this error and show warning or something like this. Or even - tns doctor should not get here in case we already know Xcode is not installed.

Plamen5kov commented 7 years ago

Yes, my plan was to add a check to tns doctor on the expected default tools on mac, just like we do here. Will fix as soon as I get the time