Plamen5kov/as compatible cli #3193
Conversation
| @@ -46,19 +48,41 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject | |||
| if (!projectData && !this._platformData) { | |||
| throw new Error("First call of getPlatformData without providing projectData."); | |||
| } | |||
| if (projectData && projectData.platformsDir) { | |||
| const projectRoot = path.join(projectData.platformsDir, AndroidProjectService.ANDROID_PLATFORM_NAME); | |||
| if (this.isAndroidStudioCompattibleTemplate(projectData)) { | |||
There was a problem hiding this comment.
Minor typo
isAndroidStudioCompattibleTemplate -> isAndroidStudioCompatibleTemplate
| @@ -38,6 +39,7 @@ export class AndroidProjectService extends projectServiceBaseLib.PlatformProject | |||
| private $npm: INodePackageManager) { | |||
| super($fs, $projectDataService); | |||
| this._androidProjectPropertiesManagers = Object.create(null); | |||
| this.isASTemplate = false; | |||
There was a problem hiding this comment.
I'm guessing we're talking about AndroidStudio template here - can we name the variable isAndroidStudioTemplate for example so that it's a bit clearer
|
|
||
| const appDestinationDirectoryArr = ["src", "main", "assets"]; | ||
| if (this.isASTemplate) { | ||
| appDestinationDirectoryArr.unshift("app"); |
There was a problem hiding this comment.
You can use APP_FOLDER_NAME from constants here
|
|
||
| const configurationsDirectoryArr = ["src", "main", "AndroidManifest.xml"]; | ||
| if (this.isASTemplate) { | ||
| configurationsDirectoryArr.unshift("app"); |
There was a problem hiding this comment.
You can use APP_FOLDER_NAME from constants here too
|
|
||
| const deviceBuildOutputArr = ["build", "outputs", "apk"]; | ||
| if (this.isASTemplate) { | ||
| deviceBuildOutputArr.unshift("app"); |
There was a problem hiding this comment.
you can use APP_FOLDER_NAME here too
| this.isASTemplate = true; | ||
| } | ||
|
|
||
| const appDestinationDirectoryArr = ["src", "main", "assets"]; |
There was a problem hiding this comment.
I have a suggestion for this code. Right now we start with 3 items in the array and later on we unshift one or two items to it. This seems a bit backwards to me.
Instead, we could start from the first item and work our way towards the end like so:
const appDestinationDirectoryArr = [projectRoot];
if (this.isAndroidStudioTemplate) {
appDestinationDirectoryArr.push(APP_FOLDER_NAME);
}
appDestinationDirectoryArr.push("src", "main", "assets");This way the logic will be start-to-end in contrast of end-to-start. In addition, push is very likely faster than unshift.
This can be done for all 3 occurrences of this type of code down below.
| } | ||
| appDestinationDirectoryArr.unshift(projectRoot); | ||
|
|
||
| const configurationsDirectoryArr = ["src", "main", "AndroidManifest.xml"]; |
There was a problem hiding this comment.
We can introduce constants for these hard-coded strings. I'm thinking something like:
export class AndroidSpecificFileNames {
static SRC_DIR = "src";
static MAIN_DIR = "main";
static MANIFEST_FILE_NAME = "AndroidManifest.xml";
static BUILD_DIR = "build";
static OUTPUTS_DIR = "outputs";
static APK_DIR = "apk";
static ASSETS_DIR = "assets";
static RES_DIR = "res";
}for example. What do you think?
There was a problem hiding this comment.
I'll add them to constants file since we already have an established commonplace for constants.
| return path.join(this.getPlatformData(projectData).projectRoot, "src", "main", "res"); | ||
| const resourcePath: string[] = ["src", "main", "res"]; | ||
| if (this.isASTemplate) { | ||
| resourcePath.unshift("app"); |
There was a problem hiding this comment.
You can use the APP_FOLDER_NAME constant here too
| resourcePath.unshift("app"); | ||
| } | ||
|
|
||
| return path.join(this.getPlatformData(projectData).projectRoot, path.join(...resourcePath)); |
There was a problem hiding this comment.
There's no need for the inner path.join statement - it could just be
return path.join(this.getPlatformData(projectData).projectRoot, ...resourcePath);| } | ||
| } | ||
|
|
||
| const asCompatibleTemplate = "3.4.0"; |
There was a problem hiding this comment.
We could name this androidStudioCompatibleTemplate
|
ping @Mitko-Kerezov |
|
run ci |
1 similar comment
|
run ci |
PR accommodates the new android runtime, project template: NativeScript/android#875
related PR: NativeScript/nativescript-dev-webpack#310