Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Ux init #633

Merged
merged 2 commits into from Sep 24, 2019
Merged

Ux init #633

merged 2 commits into from Sep 24, 2019

Conversation

rumpl
Copy link
Member

@rumpl rumpl commented Sep 24, 2019

- What I did

Made UX changes to the init subcommand:

  • removed the maintainer flag
  • removed the description flag
  • removed implicit compose file search

- How I did it

Removed a bunch of code.

- How to verify it

Run docker app init --help, the flags description and maintainer should no longer be there.

- Description for the changelog
Docker App doesn't implicitly select a docker-compose.yml file on init.
The flags --description and --maintainer heve been removed.

- A picture of a cute animal (not mandatory but encouraged)
image

As per the UX study we remove these two flags to make init simpler.
The maintainer is populated with the name of the current user.

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
The user needs to explicitly give the path to the docker-compose file
that they want to base the app on.

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Copy link
Contributor

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

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

LGTM
I like code being removed :P

Copy link
Contributor

@jcsirot jcsirot left a comment

Choose a reason for hiding this comment

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

LGTM

@jcsirot jcsirot merged commit cc0d5ee into docker:master Sep 24, 2019
@@ -76,25 +77,30 @@ func TestInit(t *testing.T) {
cmd, cleanup := dockerCli.createTestCmd()
defer cleanup()

userData, _ := user.Current()
currentUser := ""
if userData != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

var currentUser string
if userData, _ := user.Current(); userData != nil{
    currentUser = userData.Username
}

)

func initCmd(dockerCli command.Cli) *cobra.Command {
cmd := &cobra.Command{
Use: "init APP_NAME [--compose-file COMPOSE_FILE] [--description DESCRIPTION] [--maintainer NAME:EMAIL ...] [OPTIONS]",
Use: "init APP_NAME [--compose-file COMPOSE_FILE] [OPTIONS]",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rename APP_NAME with APP_DEFINITION ?

assert.NilError(t, err)

data := `# Version of the application
userData, _ := user.Current()
Copy link
Contributor

Choose a reason for hiding this comment

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

Same pattern here 😅

@jcsirot
Copy link
Contributor

jcsirot commented Sep 25, 2019

Argh, I already merged it. My bad 😢

@rumpl rumpl deleted the ux-init branch October 18, 2019 08:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants