fix (getting started): Improve behaviour when nativescript-cloud is installed #3495

Fatme · 2018-03-26T06:29:17Z

Fixes the behaviour of getting started when nativescript-cloud is already installed.
Fixes messages when console is non-interactive.
Add In case you have any issues, you can ask in our forum. message when env is not configured after setup script.
Refactor unit tests

rosen-vladimirov · 2018-03-26T19:12:11Z

lib/services/nativescript-cloud-extension-service.ts

+	}
+
+	private getExtensionData(): IExtensionData {
+		return this.$extensibilityService.getInstalledExtensionsData()


Can you please use _.find instead of array.find, it is safer to use Lodash methods

rosen-vladimirov · 2018-03-26T19:30:25Z

lib/services/platform-environment-requirements.ts

+
+		return `${PlatformEnvironmentRequirements.MISSING_LOCAL_AND_CLOUD_SETUP_MESSAGE}` + EOL
+			+ "Run $ tns setup command to run the setup script to try to automatically configure your environment for local builds." + EOL
+			+ `Run $ tns cloud setup command to install the ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension to configure your environment for cloud builds.` + EOL


this line of the message is the only difference between this message and the one used in the if above. I suggest extrating the cloud message in a const and use it in the message here. This way you'll not duplicate all other messages:

const cloudBuildMessage = this.$nativescriptCloudExtensionService.isInstalled() ? this.getCloudBuildsMessage(platform) : `Run $ tns cloud setup command to install the ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension to configure your environment for cloud builds.`;

rosen-vladimirov · 2018-03-26T19:32:59Z

lib/services/platform-environment-requirements.ts

+			+ `Select "Configure for Cloud Builds" to install the ${constants.NATIVESCRIPT_CLOUD_EXTENSION_NAME} extension and automatically configure your environment for cloud builds.` + EOL
+			+ `Select "Configure for Local Builds" to run the setup script and automatically configure your environment for local builds.`
+			+ `Select "Configure for Both Local and Cloud Builds" to automatically configure your environment for both options.`
+			+ `Select "Skip Step and Configure Manually" to disregard these options and install any required components manually.`;


again, some of the messages are duplicated, can you extract them in const variables.

rosen-vladimirov · 2018-03-26T19:36:41Z

test/services/platform-environment-requirements.ts

@@ -6,7 +6,8 @@ import { assert } from "chai";
 const platform = "android";
 const cloudBuildsErrorMessage = `In order to test your application use the $ tns login command to log in with your account and then $ tns cloud build command to build your app in the cloud.`;
 const manuallySetupErrorMessage = `To be able to build for ${platform}, verify that your environment is configured according to the system requirements described at `;
-const nonInteractiveConsoleErrorMessage = `You are missing the nativescript-cloud extension and you will not be able to execute cloud builds. Your environment is not configured properly and you will not be able to execute local builds. To continue, choose one of the following options: \nRun $ tns setup command to run the setup script to try to automatically configure your environment for local builds.\nRun $ tns cloud setup command to install the nativescript-cloud extension to configure your environment for cloud builds.\nVerify that your environment is configured according to the system requirements described at `;
+const nonInteractiveConsoleMessageWhenExtensionIsNotInstalled = `You are missing the nativescript-cloud extension and you will not be able to execute cloud builds. Your environment is not configured properly and you will not be able to execute local builds. To continue, choose one of the following options: \nRun $ tns setup command to run the setup script to try to automatically configure your environment for local builds.\nRun $ tns cloud setup command to install the nativescript-cloud extension to configure your environment for cloud builds.\nVerify that your environment is configured according to the system requirements described at `;


you can use os.EOL instead of \n
Also can you please wrap the calls to command in quotes, for example ``Run '$ tns setup' command to run the...`

rosen-vladimirov · 2018-03-26T19:36:48Z

test/services/platform-environment-requirements.ts

@@ -6,7 +6,8 @@ import { assert } from "chai";
 const platform = "android";
 const cloudBuildsErrorMessage = `In order to test your application use the $ tns login command to log in with your account and then $ tns cloud build command to build your app in the cloud.`;
 const manuallySetupErrorMessage = `To be able to build for ${platform}, verify that your environment is configured according to the system requirements described at `;
-const nonInteractiveConsoleErrorMessage = `You are missing the nativescript-cloud extension and you will not be able to execute cloud builds. Your environment is not configured properly and you will not be able to execute local builds. To continue, choose one of the following options: \nRun $ tns setup command to run the setup script to try to automatically configure your environment for local builds.\nRun $ tns cloud setup command to install the nativescript-cloud extension to configure your environment for cloud builds.\nVerify that your environment is configured according to the system requirements described at `;
+const nonInteractiveConsoleMessageWhenExtensionIsNotInstalled = `You are missing the nativescript-cloud extension and you will not be able to execute cloud builds. Your environment is not configured properly and you will not be able to execute local builds. To continue, choose one of the following options: \nRun $ tns setup command to run the setup script to try to automatically configure your environment for local builds.\nRun $ tns cloud setup command to install the nativescript-cloud extension to configure your environment for cloud builds.\nVerify that your environment is configured according to the system requirements described at `;
+const nonInteractiveConsoleMessageWhenExtensionIsInstalled = `Your environment is not configured properly and you will not be able to execute local builds. To continue, choose one of the following options: \nRun $ tns setup command to run the setup script to try to automatically configure your environment for local builds.\nIn order to test your application use the $ tns login command to log in with your account and then $ tns cloud build command to build your app in the cloud.\nVerify that your environment is configured according to the system requirements described at .`;


rosen-vladimirov · 2018-03-26T19:41:02Z

lib/services/platform-environment-requirements.ts

 	private static NOT_CONFIGURED_ENV_MESSAGE = "To continue, choose one of the following options: ";
-	private static NOT_CONFIGURED_ENV_AFTER_SETUP_SCRIPT_MESSAGE = "The setup script was not able to configure your environment for local builds. To execute local builds, you have to set up your environment manually. To continue, choose one of the following options:";
+	private static NOT_CONFIGURED_ENV_AFTER_SETUP_SCRIPT_MESSAGE = "The setup script was not able to configure your environment for local builds. To execute local builds, you have to set up your environment manually. In case you have any issues, you can ask in our forum. To continue, choose one of the following options:";


we need the actual link to the forum here: http://forum.nativescript.org

…lready installed. Fixes messages when console is non-interactive. Add `In case you have any issues, you can ask in our forum.` message when env is not configured after setup script. Refactor unit tests

Fatme · 2018-03-29T19:21:53Z

run ci

Fatme · 2018-04-02T07:51:09Z

run ci

rosen-vladimirov suggested changes Mar 26, 2018

View reviewed changes

Fatme force-pushed the fatme/fix-env-messages branch from c5457ea to 97f550e Compare March 29, 2018 12:45

Fatme added 3 commits March 29, 2018 15:47

[WIP] Fix env messages

ad23f8c

Fixes the behaviour of getting started when nativescript-cloud is a…

28a0cbd

…lready installed. Fixes messages when console is non-interactive. Add `In case you have any issues, you can ask in our forum.` message when env is not configured after setup script. Refactor unit tests

Fix PR comments

9940fdd

Fatme force-pushed the fatme/fix-env-messages branch from 97f550e to 9940fdd Compare March 29, 2018 12:47

Rename nativescript to nativeScript

50bbfb6

Fatme force-pushed the fatme/fix-env-messages branch from 2168c55 to 50bbfb6 Compare April 2, 2018 06:29

rosen-vladimirov approved these changes Apr 2, 2018

View reviewed changes

Fatme merged commit f1a2995 into release Apr 3, 2018

Fatme deleted the fatme/fix-env-messages branch April 3, 2018 08:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix (getting started): Improve behaviour when nativescript-cloud is installed #3495

fix (getting started): Improve behaviour when nativescript-cloud is installed #3495

Fatme commented Mar 26, 2018

rosen-vladimirov Mar 26, 2018

rosen-vladimirov Mar 26, 2018

rosen-vladimirov Mar 26, 2018

rosen-vladimirov Mar 26, 2018

rosen-vladimirov Mar 26, 2018

rosen-vladimirov Mar 26, 2018

Fatme commented Mar 29, 2018

Fatme commented Apr 2, 2018

fix (getting started): Improve behaviour when nativescript-cloud is installed #3495

fix (getting started): Improve behaviour when nativescript-cloud is installed #3495

Conversation

Fatme commented Mar 26, 2018

rosen-vladimirov Mar 26, 2018

Choose a reason for hiding this comment

rosen-vladimirov Mar 26, 2018

Choose a reason for hiding this comment

rosen-vladimirov Mar 26, 2018

Choose a reason for hiding this comment

rosen-vladimirov Mar 26, 2018

Choose a reason for hiding this comment

rosen-vladimirov Mar 26, 2018

Choose a reason for hiding this comment

rosen-vladimirov Mar 26, 2018

Choose a reason for hiding this comment

Fatme commented Mar 29, 2018

Fatme commented Apr 2, 2018