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

fix: omit double quotes from connection name #983

Merged
merged 2 commits into from
Mar 8, 2024

Conversation

pokedpeter
Copy link
Contributor

@pokedpeter pokedpeter commented Jan 16, 2024

When passing connection name parameter, connection name includes double quotes, so omit them

fix #947

πŸ”— Linked issue

#947

❓ Type of change

  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Some Ace migration commands pass the connection name parameter to other commands including double quotes, which results in an error.

This fix excludes the double quotes when passing through the connection name string.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

When passing connection name parameter, connection name includes double quotes, so omit them

fix adonisjs#947
@pokedpeter
Copy link
Contributor Author

One of the PR checks failed due to a failed npm install. Couldn't see a way to trigger the checks again.

image

@RomainLanz RomainLanz closed this Feb 15, 2024
@RomainLanz RomainLanz reopened this Feb 15, 2024
@RomainLanz RomainLanz merged commit 8c333e0 into adonisjs:develop Mar 8, 2024
31 of 32 checks passed
@RomainLanz
Copy link
Member

Thanks!

@thetutlage
Copy link
Member

It will be nice to have tests alongside it, because looking at the issue, I see the connection name already had double quotes and since wrapping it inside double quotes again caused the issue.

But ideally, there shouldn't been any double quotes in first place.

So let's have tests around it.

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

Successfully merging this pull request may close these issues.

Ace migration commands failing when using the connection flag
3 participants