Azure / azure-dev

A developer CLI that reduces the time it takes for you to get started on Azure. The Azure Developer CLI (azd) provides a set of developer-friendly commands that map to key stages in your workflow - code, build, deploy, monitor, repeat.
https://aka.ms/azd
MIT License
373 stars 168 forks source link

[Issue] azd init fails after declining to overwrite files from template repo #1656

Closed savannahostrowski closed 1 year ago

savannahostrowski commented 1 year ago

Output from azd version Run azd version and copy and paste the output here: azd version 0.6.0-beta.2 (commit c4a201e55a13eba77f9037b296a90e0f81b00dbe)

Describe the bug When I decline to let azd overwrite the .gitignore, LICENSE, and README.md in my current project, it failed to initialize.

image

To Reproduce

  1. Open an existing project (non-azd compatible)
  2. Run azd init on the codebase
  3. Choose a template from the list
  4. Decline letting azd overwrite your files

Expected behavior This should succeed. I'd expect that the template files are added to my project and the .gitignore, LICENSE and README.md are untouched by azd.

In this case, none of these files are even core to executing the template. They're metadata and configuration.

weikanglim commented 1 year ago

The intention was probably to avoid the user from cloning into an existing repository by accident.

It seems that a simple change could be to update the prompt:

savannahostrowski commented 1 year ago

Yeah - that makes sense. I think this would avoid us having a blocking UX. The copy might need some finessing.

cc: @Austinauth for visibility and in case he has anything to add cc: @ellismg as FYI

Austinauth commented 1 year ago

@savannahostrowski @vhvb1989 @weikanglim

Currently investigating more accessible list select patterns, in the interim we can keep the current pattern (selecting with a >), unless we feel strongly about using the letters ( o), k), c) ).

image

savannahostrowski commented 1 year ago

I think that looks great tbh. I agree with keeping our styles consistent as you've outline. Super clear.

savannahostrowski commented 1 year ago

@weikanglim @rajeshkamal5050 It'd be great if we could prioritize a fix for this. Right now, it sort of hinders me from doing azdev-ifying in demos.

weikanglim commented 1 year ago

@Austinauth @savannahostrowski Is Abort init maybe slightly better than Cancel init? I'm trying to convey the idea that nothing will happen, so that the user isn't left wondering if they're in an intermediate state where azd performed something, but not quite all of it.

weikanglim commented 1 year ago

Perhaps it's clear from the logging what azd did, since any modifications would be logged, and the absence of that suggests the same idea. I'm going to move forward with Cancel, but just curious if you have a quick feeling one way or another.

Austinauth commented 1 year ago

I don't have a strong feeling on the wording one way or another, I personally I feel both abort and cancel convey the same thing in this scenario.

Super quick sketch, wording probably needs adjustment.In the future we might be able to do something like this to be more clear. We do not currently have a pattern like this implemented.

image

savannahostrowski commented 1 year ago

Is there any reason that we can't use the same style we use for region or subscription selection here?

savannahostrowski commented 1 year ago

"Cancel init" is fine, IMO.

Austinauth commented 1 year ago

Is there any reason that we can't use the same style we use for region or subscription selection here?

That's what's proposed above: https://github.com/Azure/azure-dev/issues/1656#issuecomment-1458633571

Sketch is just to illustrate how we could add additional context if required.

weikanglim commented 1 year ago

@Austinauth @savannahostrowski I realized I had made a faulty suggestion by putting "Cancel init" as an option.

The typical way the user would "cancel" out of a CLI is to use "Ctrl + C" on their keyboard. This does work here. I'm worried if we introduce Cancellation as an option, we might be stuck in trying to implement this UI pattern everywhere, so I would maybe suggest to not introduce that options unless we need to?

savannahostrowski commented 1 year ago

Yeah - that's true that people can do ctrl/cmd+c to cancel out and a fair concern about how this scales. I agree - we shouldn't introduce a cancel option unless we have supporting data to back doing so. I'd expect that most know that ctrl+c would get them out of the command execution.

weikanglim commented 1 year ago

I have a change out to switch the confirmation to a selection.

@Austinauth @savannahostrowski I think we might also need to reword the yellow warning message below, which states "The following files will be overwritten with the versions from the template":

image

I can't find the words to best express this. My really poor attempt looks like:

The following files are present both locally and in the template:

savannahostrowski commented 1 year ago

"The template contains files with the same name as files in your application:

What would you like to do? ... "

I don't love that wording either :D

Alternatively, we could take a more "merge conflict" route and borrow verbiage (which could be familiar) from git? And use the term "conflicts"?

Austinauth commented 1 year ago

I think both suggestions are fine, I added a few more options.

  1. The following files are present both locally and in the template:
  2. The template contains files with the same name as files in your application:
  3. The template you've selected contains files with the same name as files in your application:
  4. Both the template and your local directory contain the following files:

All are serviceable IMO.

We should probably tweak the prompt choices as well, perhaps:

weikanglim commented 1 year ago

I like The following files are present both locally and in the template, and the prompt choice tweaks makes sense to me. Going with those options in my PR.

savannahostrowski commented 1 year ago

My only comment with "locally" is just for us to keep in mind that someone might do this from Codespaces. Does locally still feel like the right word here?

weikanglim commented 1 year ago

It makes sense to me. I think of Cloudspaces as hosted in the cloud, but when I'm in the VM, I treat it just as any other machine. I maybe wouldn't expect azd to call out that I'm running in Codespaces (logically seems reversed of how it would be).

I didn't pick the text with "application" wording, because it sounds a little vague. When I'm initializing an azd project/application, I don't know if I'd expect my code before azd init runs to be called an application.