Skip to content

Fix broken install command#470

Merged
harrysarson merged 5 commits intortfeldman:masterfrom
lydell:fix-install-command
Nov 17, 2020
Merged

Fix broken install command#470
harrysarson merged 5 commits intortfeldman:masterfrom
lydell:fix-install-command

Conversation

@lydell
Copy link
Collaborator

@lydell lydell commented Nov 15, 2020

Regression from #465.

Before:

❯ ../bin/elm-test install truqu/elm-md5
I am having trouble with this argument:

    t

It is supposed to be a <package> value, like one of these:

    elm/html
    elm/http
    elm/svg
    elm/time

After:

❯ ../bin/elm-test install truqu/elm-md5
Here is my plan:

  Add:
    truqu/elm-md5            1.1.0
    zwilias/elm-utf-tools    2.0.1

Would you like me to update your elm.json accordingly? [Y/n]:
Success!

This happened because I thought the first argument of the .action callback is always an array. Wrong! The signature depends on the .command syntax provided:

program
  .command('name <one> <two> [three] [rest...]')
  .action((one: string, two: string, three: string | undefined, rest: Array<string>, cmd: Command) => {
    // Do stuff
  });

For the install command we have:

program
  .command('install <package>')
  .action((packageName: string, cmd: Command) => {})

Before we tried to destructure the first argument (thinking it was an array), so we only got the first letter of the package name.

Unfortunately our test suite doesn’t catch this.

@harrysarson
Copy link
Collaborator

Nice catch! Could you add a regression test?

@lydell
Copy link
Collaborator Author

lydell commented Nov 15, 2020

Do you have any ideas on how to write one?

@lydell
Copy link
Collaborator Author

lydell commented Nov 15, 2020

I figured it out.

@harrysarson
Copy link
Collaborator

It looks like travis has pulled macos support. I guess we cannot complain too much because we were getting it for free.

@lydell could you remove mac from travis just so we can get a green tick. Longer time we probably want to migrate to github actions.

Cc: @rtfeldman
Refs: https://blog.travis-ci.com/2020-11-02-travis-ci-new-billing

@harrysarson harrysarson added the status: blocked We cannot make anymore progress here until something else gets done label Nov 15, 2020
@harrysarson harrysarson merged commit dbc2d18 into rtfeldman:master Nov 17, 2020
@lydell lydell deleted the fix-install-command branch November 17, 2020 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: blocked We cannot make anymore progress here until something else gets done

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants