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.

Sign up for GitHub

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

Jump to bottom

Fix completed migration listing #5060

Merged
merged 1 commit into from Mar 19, 2022
Merged

Fix completed migration listing #5060

merged 1 commit into from Mar 19, 2022

Conversation

santialbo
Copy link
Contributor

Fixes #4951

OlivierCavadenti
OlivierCavadenti reviewed Mar 14, 2022
View reviewed changes
Copy link
Collaborator

@OlivierCavadenti OlivierCavadenti left a comment

Choose a reason for hiding this comment

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

thx for the PR. No tests are changed ? Can you add one that fail before the change and success after ?

@santialbo
Copy link
Contributor Author

thx for the PR. No tests are changed ? Can you add one that fail before the change and success after ?

Done! Updated an existing test for better coverage. This test without the change would have failed.

@santialbo
Copy link
Contributor Author

santialbo commented Mar 14, 2022 •
edited

It seems I'm having some trouble with ansi color codes here :/
For some reason on my local env, they don't show up on the stdout but they do here on the CI containers.

Edit: I had to add the NO_COLOR env variable when calling the execCommand to get consistent output.

@kibertoad kibertoad merged commit 1d4cd0b into knex:master Mar 19, 2022
@kibertoad
Copy link
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OlivierCavadenti OlivierCavadenti OlivierCavadenti left review comments

@kibertoad kibertoad kibertoad approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

npx knex migrate:list doesn't print a readable list of completed migrations files
3 participants
@santialbo @kibertoad @OlivierCavadenti