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

add createSchema and dropSchema methods on MigrationInterface #1871

Merged
merged 1 commit into from
Sep 28, 2020
Merged

add createSchema and dropSchema methods on MigrationInterface #1871

merged 1 commit into from
Sep 28, 2020

Conversation

MasterOdin
Copy link
Member

Closes #1870

Given that there's no docs right now for createDatabase either, did not elect to add them here (as not sure where to place them). For now, it should at least make it a bit easier for user discovery.

PR made against 0.13 due to breaking change on the MigrationInterface

Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
@MasterOdin MasterOdin changed the title add createSchema method on MigrationInterface add createSchema and dropSchema methods on MigrationInterface Aug 23, 2020
@dereuromark dereuromark added this to the 0.13 milestone Aug 26, 2020
* @return void
* @throws \BadMethodCallException
*/
public function createSchema($name);
Copy link
Member

Choose a reason for hiding this comment

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

If we add those freshly, couldnt we add the typehints? Or should we keep them off to be in sync with the other methods?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would say that the functions type hinting within the interface be consistent throughout the file, either all or nothing.

I do support in 0.13 to make the BC break here and add the type hints throughout the interface, especially given the larger BC break in #1872 already potentially going into 0.13. Alternatively, could hold off till 0.14 (or some future version), though I do think it should be done at some point.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, makes sense
What do others think?

Copy link
Member

Choose a reason for hiding this comment

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

So merging now, but I agree that 0.13 could and should add those typehints.

@dereuromark dereuromark merged commit 1a05e29 into cakephp:0.next Sep 28, 2020
@Bilge
Copy link
Contributor

Bilge commented Jan 3, 2023

It seems this is not available in seed files.

@MasterOdin
Copy link
Member Author

The createDatabase and dropDatabase methods are also not available on the SeedInterface.

Just for my own edification, what's the use-case on wanting to create / drop schemas as part of seeding, and not the migration process?

@Bilge
Copy link
Contributor

Bilge commented Jan 3, 2023

See #2161.

Aside, I don't understand why seeding and migration don't share the same API.

@MasterOdin
Copy link
Member Author

For #2161, the end result of what you'd want is that:

  1. phinx itself doesn't care about the migration table's existence when running seeds
  2. that you would have a method to easily create the schema for the phinx migration table

Aside, I don't understand why seeding and migration don't share the same API.

Agreed now that I've looked at them more closely.

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

3 participants