Skip to content

Fix docs example for migrate create #2284

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
Merged
MasterOdin merged 1 commit into from
Jun 4, 2024
Merged

Fix docs example for migrate create #2284

MasterOdin merged 1 commit into from
Jun 4, 2024

Conversation

@MasterOdin
Copy link
Member

@MasterOdin MasterOdin commented Jun 4, 2024 •
edited
Loading

This reverts commit 1187a5b.

From #2263 (comment):

When you provide a class name, then we hit this line to get the filename:

phinx/src/Phinx/Console/Command/Create.php

Line 183 in 5c28445

$fileName = Util::mapClassNameToFileName($className);

which per the comment:

phinx/src/Phinx/Util/Util.php

Lines 103 to 105 in 5c28445

* Turn migration names like 'CreateUserTable' into file names like
* '12345678901234_create_user_table.php' or 'LimitResourceNamesTo30Chars' into
* '12345678901234_limit_resource_names_to_30_chars.php'.

The tests for this function also back up the behavior here:

phinx/tests/Phinx/Util/UtilTest.php

Lines 71 to 86 in 5c28445

public function providerMapClassNameToFileName(): array
{
return [
['CamelCase87afterSomeBooze', '/^\d{14}_camel_case_87after_some_booze\.php$/'],
['CreateUserTable', '/^\d{14}_create_user_table\.php$/'],
['LimitResourceNamesTo30Chars', '/^\d{14}_limit_resource_names_to_30_chars\.php$/'],
];
}
/**
* @dataProvider providerMapClassNameToFileName
*/
public function testMapClassNameToFileName(string $name, string $pattern): void
{
$this->assertMatchesRegularExpression($pattern, Util::mapClassNameToFileName($name));
}

I think the capitalization that the docs was changed to might be a cakephp/migrations thing and not what phinx itself does?

@MasterOdin MasterOdin requested a review from dereuromark June 4, 2024 03:31
@dereuromark
Copy link
Member

dereuromark commented Jun 4, 2024

Interesting. Might be then yeah
While it makes sense to have the name closer to the class name I guess this is a breaking change.
Feel free to merge

@MasterOdin
Copy link
Member Author

MasterOdin commented Jun 4, 2024

We could certainly make this a feature flag if it's desired behavior, though at that point, might be better to just go for filenames that adhere to PSR-4.

@MasterOdin MasterOdin merged commit a31003d into 0.x Jun 4, 2024
@MasterOdin MasterOdin deleted the docs-migration-create branch June 4, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dereuromark dereuromark Awaiting requested review from dereuromark

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MasterOdin @dereuromark