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

migrate foo invalid command is not an error #157

Closed
hdon opened this issue May 21, 2019 · 7 comments
Closed

migrate foo invalid command is not an error #157

hdon opened this issue May 21, 2019 · 7 comments

Comments

@hdon
Copy link

hdon commented May 21, 2019

Giving migrate an invalid command does not issue an error and exits normally:

hdon@ubuntu-s-1vcpu-1gb-sfo2-01:~/migrate-project$ npm i migrate
npm WARN saveError ENOENT: no such file or directory, open '/home/hdon/migrate-project/package.json'
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN enoent ENOENT: no such file or directory, open '/home/hdon/migrate-project/package.json'
npm WARN migrate-project No description
npm WARN migrate-project No repository field.
npm WARN migrate-project No README data
npm WARN migrate-project No license field.

+ [email protected]
added 20 packages from 13 contributors and audited 21 packages in 1.15s
found 0 vulnerabilities

hdon@ubuntu-s-1vcpu-1gb-sfo2-01:~/migrate-project$ ./node_modules/.bin/migrate init
  migrations dir : /home/hdon/migrate-project/migrations
hdon@ubuntu-s-1vcpu-1gb-sfo2-01:~/migrate-project$ ./node_modules/.bin/migrate make first_migration
  migration : complete
hdon@ubuntu-s-1vcpu-1gb-sfo2-01:~/migrate-project$ echo $?
0

Note: migrate make first_migration is not a valid migrate command.

@wesleytodd
Copy link
Collaborator

Hi @hdon, make is not a command in this tool. To see how it works I suggest using migrate --help. Good luck!

@LinusU
Copy link
Collaborator

LinusU commented May 21, 2019

@wesleytodd I think that the issue was that migrate didn't raise an error, and exited with a 0 status code, instead of raising an error and setting a failure status code ☺️

Opening for now, but feel free to close if I'm wrong 🐎

@LinusU LinusU reopened this May 21, 2019
@wesleytodd
Copy link
Collaborator

Right, but this is related to the other issue, because what it is running is the "default" command up, but make is not a valid migration. So because previously that was not an error case it would not exit with an error code.

That being said, I am not sure we have good test coverage of exit codes, so that might be a good area to look at.

@hdon
Copy link
Author

hdon commented May 22, 2019

@LinusU is correct.

I chose the example of "make" because it's the exact mistake I made to discover the confusing behavior.

@wesleytodd so in addition to not erroring, it also migrated all the way up? 😱

@wesleytodd
Copy link
Collaborator

wesleytodd commented May 22, 2019

Yeah, so the fix is to error when you try to run with a migration name which does not exist. We can close this in favor of #156, because it is really the same issue.

@hdon
Copy link
Author

hdon commented May 23, 2019

Yeah, that would be better. Though IMHO it's better for tools to ask you to be more explicit. I think defaulting to "up" is not really saving anyone any effort but some will get tripped up by the automatic "up" behavior.

@wesleytodd
Copy link
Collaborator

@hdon I agree, maybe we should also change that behavior. That was also the pre-1.0 behavior. Want to open a new issue to make that recommendation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants