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

Fixing Namespace Option Ending with Backslash in LiftMigration Command #85

Conversation

BadJacky
Copy link
Contributor

This PR addresses an issue in the LiftMigration command where the namespace option could end with a backslash (\\). In the original code, if the namespace option ended with a backslash, it would result in two consecutive backslashes when concatenating the model name. This could lead to unexpected issues.

In this PR, I've used the rtrim function to ensure that the namespace option does not end with a backslash before concatenating the model name.

Here's the modified line of code:

$class = rtrim($this->option('namespace'), '\\') . '\\' . $this->argument('model'); // @phpstan-ignore-line

This change has passed all tests, including a new test case specifically designed to check if a migration file is correctly generated when the namespace option ends with two backslashes (\\).

Please review this PR and feel free to provide any feedback or suggestions.

@BadJacky BadJacky force-pushed the bugfix/fix-namespace-concatenation-with-duplicate-slash branch from 8257ef0 to 17f8dba Compare May 28, 2024 15:50
@BadJacky BadJacky force-pushed the bugfix/fix-namespace-concatenation-with-duplicate-slash branch from 17f8dba to 2e593f2 Compare May 28, 2024 15:54
Copy link
Owner

@WendellAdriel WendellAdriel left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!!! 🔥 💪

@WendellAdriel WendellAdriel merged commit 1c3b292 into WendellAdriel:main May 28, 2024
5 checks passed
@BadJacky BadJacky deleted the bugfix/fix-namespace-concatenation-with-duplicate-slash branch May 29, 2024 13:09
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.

None yet

2 participants