Skip to content
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

Remove reliance on func cli for 'Create new project' #63

Merged
merged 4 commits into from
Nov 10, 2017
Merged

Conversation

ejizba
Copy link
Contributor

@ejizba ejizba commented Nov 9, 2017

Benefits:

  1. User can now create project and function without func cli (they only need the cli for debugging)
  2. We can now prompt the user to overwrite existing files
  3. We can automatically detect if we should git init the folder

This results in a little bit of copied logic between us and the func cli, but the benefits outweigh that drawback

Fixes #24
Fixes #21 (since func cli is not required until debugging anymore)
Fixes #18 (since func cli output isn't logged in the output window anymore)

Benefits:
1. User can now create project and function without func cli (they only need the cli for debugging)
1. We can now prompt the user to overwrite existing files
1. We can automatically detect if we should git init the folder
@ejizba ejizba requested a review from a team November 9, 2017 21:40
@@ -1,6 +1,6 @@
# Azure Functions for Visual Studio Code (Preview)

## Prerequisites
## Prerequisites for local debugging
* [Node v8.0+](https://nodejs.org/)
* Older versions of node will be supported soon. See issue [#1](https://github.com/Microsoft/vscode-azurefunctions/issues/1)
* [.NET Core 2.0](https://www.microsoft.com/net/download/core)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is true? I had thought apps were supposed to install .net core libs with their app.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep: https://docs.microsoft.com/azure/azure-functions/functions-run-local

They're supposed to bundle .net core libs with their app, but they haven't done that quite yet (After all, the cli is still technically in preview)

} else {
async function validateIsFunctionApp(outputChannel: vscode.OutputChannel, functionAppPath: string, ui: IUserInterface): Promise<void> {
if (requiredFunctionAppFiles.find((file: string) => !fse.existsSync(path.join(functionAppPath, file))) !== undefined) {
const message: string = localize('azFunc.notFunctionApp', 'The current folder is not a function app project. Initialize Project?');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selected folder?

src/utils/fs.ts Outdated

export function isSubPath(fsPath1: string, fsPath2: string): boolean {
const relativePath: string = path.relative(fsPath1, fsPath2);
return relativePath !== '' && !relativePath.startsWith('..');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: seems like !== '' not necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remove that check, then you get 'true' if the paths are the same.

It doesn't seem like /example/path/ is a subfolder of /example/path/

src/utils/fs.ts Outdated
return relativePath === '';
}

export function isSubPath(fsPath1: string, fsPath2: string): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please comment which is being checked as parent and which as child.

import * as assert from 'assert';
import * as fsUtils from '../src/utils/fs';

suite('fsUtils Tests', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to test with "c:", etc., and paths without a common prefix

@ejizba ejizba merged commit 8cdb675 into master Nov 10, 2017
@ejizba ejizba deleted the ej/newproject branch November 10, 2017 00:26
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants