Support create java function project #51

jdneo · 2017-11-03T02:56:13Z

Work Item: #48

Support create java function project. Currently we will use Maven archetype (call mvn cli) to create a Java typed function project. And we'll change it to function cli when the related part in cli is ready.

UI:

Todo:

Add Java function (Support add Java Function into Java Function Project #49)
deploy Java function (Support deploy Java Function #50)

-------------- Update on Wed Nov 08 2017 09:59:31 GMT+0800 (China Standard Time) --------------

Add code to support add Java function.

Using Maven plugin to do this job.
Confirmed from Maven plugin team, this command has no batch mode, so I send it to terminal and user can fill in the parameter in the terminal
Currently, we determine whether the project is Java based or not by finding pom.xml in the selected folder.

ejizba

This doesn't seem useful until we also support Java functions. Just want to make sure you have a plan to 'disable' this feature if Java function support isn't ready in time for our next release? Should be easy, right?

ejizba · 2017-11-05T17:55:52Z

src/commands/createNewProject.ts

+    switch (language.label) {
+        case TemplateLanguage.Java:
+            const groupIdPlaceHolder: string = localize('azFunc.java.groupIdPlaceholder', 'groupId');
+            const groupIdPrompt: string = localize('azFunc.java.groupIdPrompt', 'Provide value of "groupId"');


These should be 'Provide a value for' instead of 'of'. In addition, there's no need to put 'groupId' in quotes

ejizba · 2017-11-05T18:08:14Z

src/commands/createNewProject.ts

+            const artifactIdprompt: string = localize('azFunc.java.artifactIdPrompt', 'Provide value of "artifactId"');
+            const artifactId: string = await ui.showInputBox(artifactIdPlaceHolder, artifactIdprompt, false);
+
+            const versionPlaceHolder: string = localize('azFunc.java.versionPlaceHolder', '1.0-SNAPSHOT');


In general, the placeholder should match the prompt, not the default value

ejizba · 2017-11-05T18:10:01Z

src/commands/createNewProject.ts

+            const appNamePrompt: string = localize('azFunc.java.appNamePrompt', 'Provide value of "appName"');
+            const appName: string = await ui.showInputBox(appNamePlaceHolder, appNamePrompt, false, undefined, `${artifactId}-${Date.now()}`);
+
+            // Use maven command currently, will change to function CLI when the function CLI init project command is ready.


I actually prefer this over using the function cli. All of the core logic is still encapsulated in the mvn archetypeArtifact, but now we get these benefits:

A user can create a Java function project without the func cli installed.

We don't have to wait for the func cli to be updated every time we want to change this

(Plus the func cli doesn't report exit codes correctly or display parse-able output - although both of those should be fixed)

Currently, the function CLI doesn't support create Java function project yet. Our plan is leverage the Maven archetype here first. And when the function CLI part is ready, we can change the implementation to using CLI. :)

BTW, according to the newest disscussion with CLI team, it is most likely that the java function project creation part in CLI will leveraging Maven plugin also. So I think we can change to CLI in the future. Anyway we can discuss wether to introduce CLI here in the future :)

ejizba · 2017-11-05T18:10:45Z

src/commands/createNewProject.ts

-        await fsUtil.writeFormattedJson(tasksJsonPath, TemplateFiles.tasksJson);
-        await fsUtil.writeFormattedJson(launchJsonPath, TemplateFiles.launchJson);
+        const vsCodePath: string = path.join(functionAppPath, '.vscode');
+        if (!await fse.pathExists(vsCodePath)) {


fse.ensureDir does the same as these two commands

ejizba · 2017-11-05T18:12:40Z

src/functions-cli.ts

    await new Promise((resolve: () => void, reject: (e: Error) => void): void => {
        const options: cp.SpawnOptions = {
            cwd: workingDirectory,
            shell: true
        };
-        const childProc: cp.ChildProcess = cp.spawn('func', args, options);
+        const command: string = languageName === TemplateLanguage.Java ? 'mvn' : 'func';


executeCommand could be used for things other than creating a project. Let's pass in 'command' instead of 'languageName'

ejizba · 2017-11-05T18:32:55Z

src/commands/createNewProject.ts

+
+    let javaTargetPath: string = '';
+    switch (language.label) {
+        case TemplateLanguage.Java:


nit: Can we move the Java-specific logic into a separate function? I would find it more readable

ejizba · 2017-11-05T18:40:45Z

src/template-files.ts

+            presentation: {
+                reveal: 'always'
+            },
+            problemMatcher: [


A lot of this code is copied. Can we encapsulate it? At least the problemMatcher

ejizba · 2017-11-05T18:44:00Z

src/template-files.ts

+            taskName: 'Launch Function App',
+            identifier: 'launchFunctionApp',
+            linux: {
+                command: 'sh -c "mvn clean package && func host start --script-root %path%"'


Create a separate 'cleanFunctionApp' task and use 'dependsOn' in this task. That way you don't have to define separate 'linux', 'osx', and 'windows' commands

Is 'mvn clean package' necessary before every F5? If so, this might not work since it's a background task (When the user detaches the debugger, func host start is still running in the background...it doesn not get executed again the next time the user F5's)

Yes mvn clean package is necessary before every F5.

After trying locally, it's exactly as what you said. the preLaunchTask doesn't get executed again. I'm trying to figure out if there is any work around on this. Do you have any suggestions on this?
Currently the user can only hit ctrl+c to manually terminate the function host and hit F5 again.

ejizba · 2017-11-05T18:54:50Z

src/template-files.ts

+    return JSON.stringify(data, null, '    ');
+}
+
+export function getTasksJson(language: string, args: string): string {


I would prefer if getTasksJson and getLaunchJson both returned the same type of string (formatted vs unformatted). Doing a stringifyJSON and replace seems inefficient, so I would lean towards returning unformatted strings. (Just modify the object directly - something like taskJsonForJava.tasks[0].command = "func host start --script-root" + scriptRoot)

Just modify the object is good for me.

So shall we define the structure of the task json object here? Otherwise the object field cannot be accessed by .. Or we use [] to access the field, but this may raise tslint error.

Whatever works for you - I care more about making the output consistent than how you fix it

In addition, don't feel like you have to follow every tslint rule. This might be a case where you just ignore the rule

Thanks for the suggestion.

ejizba · 2017-11-05T19:16:45Z

src/utils/workspace.ts


        const folder: vscode.WorkspaceFolder | undefined = vscode.workspace.workspaceFolders.find((f: vscode.WorkspaceFolder): boolean => {
-            return fsPath.startsWith(f.uri.fsPath);
+            return path.relative(fsPath, f.uri.fsPath) === '';


Hmm - this definitely fixes some edge cases, but it's not quite what I originally intended the behavior to be. This function should return true even if the path is a subfolder of a workspace. Maybe something like this would work?:

const relativePath: string = path.relative(fsPath, f.uri.fsPath); return relativePath === '' || !relativePath.startsWith('..');

This logic can be tricky with lots of edge cases, so I'd prefer to add some unit tests when we fix this. You can do that in this PR or file an issue to fix this separately (I don't mind fixing this myself)

My concern is if the subfolder is not opened, the user might not able to launch it since the cwd might not have .vscode folder which contains launch.json and task.json.

Another concern is that as what we discussed above. The maven archetype will always create a artifactId folder first, and put function things into the folder. For Java, there is another important file: pom.xml, will also be put into the artifactId folder. The maven command can be execute only if the cwd contains pom.xml or specify its location in parameter. That's why we prefer to open the folder which contains pom.xml (that is the artifactId folder).

I quite agree that always open the project base path might not be the best solution for this. While I think dong this it not that harmful. What about keeping this change for now and file a issue and we can do more further discussion in detail?

Yeah filing an issue is fine

jdneo · 2017-11-10T08:28:38Z

Close this PR since the code structure has changed largely. See #66

jdneo force-pushed the cs-create-java-project branch 2 times, most recently from 8478e55 to 1e5197e Compare November 3, 2017 04:11

ejizba suggested changes Nov 6, 2017

View reviewed changes

jdneo force-pushed the cs-create-java-project branch 4 times, most recently from a7cbe1b to cc8d3b7 Compare November 8, 2017 02:11

jdneo added 3 commits November 8, 2017 16:19

support create java function project

6e2e839

modify according to comments

ac0b20f

can add Java function to the function project

dcaa5ef

jdneo force-pushed the cs-create-java-project branch from cc8d3b7 to dcaa5ef Compare November 8, 2017 08:23

jdneo closed this Nov 10, 2017

jdneo deleted the cs-create-java-project branch February 5, 2018 09:26

github-actions bot locked and limited conversation to collaborators Jan 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support create java function project #51

Support create java function project #51

jdneo commented Nov 3, 2017 •

edited

Loading

ejizba left a comment

ejizba Nov 5, 2017

jdneo Nov 7, 2017

ejizba Nov 5, 2017

jdneo Nov 7, 2017

ejizba Nov 5, 2017

jdneo Nov 7, 2017 •

edited

Loading

ejizba Nov 5, 2017

jdneo Nov 7, 2017

ejizba Nov 5, 2017

jdneo Nov 7, 2017

ejizba Nov 5, 2017

jdneo Nov 7, 2017

ejizba Nov 5, 2017

jdneo Nov 7, 2017

ejizba Nov 5, 2017

jdneo Nov 7, 2017

ejizba Nov 5, 2017

jdneo Nov 7, 2017

ejizba Nov 7, 2017

jdneo Nov 7, 2017

ejizba Nov 5, 2017

jdneo Nov 7, 2017 •

edited

Loading

ejizba Nov 7, 2017

jdneo Nov 7, 2017 •

edited

Loading

jdneo commented Nov 10, 2017

Support create java function project #51

Support create java function project #51

Conversation

jdneo commented Nov 3, 2017 • edited Loading

ejizba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdneo Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdneo Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdneo Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

jdneo commented Nov 10, 2017

jdneo commented Nov 3, 2017 •

edited

Loading

jdneo Nov 7, 2017 •

edited

Loading

jdneo Nov 7, 2017 •

edited

Loading

jdneo Nov 7, 2017 •

edited

Loading