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 "from-empty-schema" option for "diff" command #981

Merged
merged 1 commit into from
Jun 21, 2020
Merged

Add "from-empty-schema" option for "diff" command #981

merged 1 commit into from
Jun 21, 2020

Conversation

guilliamxavier
Copy link
Contributor

Q A
Type feature
BC Break no
Fixed issues #820

Summary

Allow to execute migrations:diff --from-empty-schema
(as alternative to migrations:dump-schema but using the mapping instead of the database)

@guilliamxavier
Copy link
Contributor Author

Not tested yet (hence draft) but WDYT?

@guilliamxavier
Copy link
Contributor Author

Update: I have tested the patch locally and I confirm it works as expected 😃 (but writing an automated test would be another story...)

@guilliamxavier guilliamxavier marked this pull request as ready for review June 8, 2020 09:09
@goetas
Copy link
Member

goetas commented Jun 13, 2020

I like this change!

A test should test the createEmptySchema method asserting something on the expected output.
(the current changes to the tests are just making sure that args are read as expected)

@guilliamxavier
Copy link
Contributor Author

guilliamxavier commented Jun 14, 2020

@goetas: Thanks, I have added tests (but I'm not sure because I couldn't mock the new Schema(...) in DiffGenerator::createEmptySchema()...)

@goetas goetas modified the milestone: 3.1.0 Jun 15, 2020
@guilliamxavier guilliamxavier marked this pull request as draft June 18, 2020 18:07
@guilliamxavier
Copy link
Contributor Author

I don't know why the "continuous-integration/travis-ci" check is blocked at "Expected — Waiting for status to be reported"... you can see that it passed: https://travis-ci.org/github/doctrine/migrations/builds/700372177

Anyway, this is now ready for review @goetas 🙂

@guilliamxavier guilliamxavier marked this pull request as ready for review June 20, 2020 14:43
@greg0ire greg0ire closed this Jun 20, 2020
@greg0ire greg0ire reopened this Jun 20, 2020
@greg0ire
Copy link
Member

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

How to do that?

  1. git rebase -i origin/2.3.x, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin/whatever-you-call-it repository.
  2. A window will show up with many lines, replace pick with fixup on every line but the first one
  3. Close your editor, git should do its magic, and you should end up with one commit
  4. Use git push --force to overwrite what you already push. Don't forget the --force option otherwise git will try to merge both things together.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Can you please add some documentation?

@guilliamxavier
Copy link
Contributor Author

@greg0ire: done and done ;)

@goetas goetas merged commit 1997e45 into doctrine:2.3.x Jun 21, 2020
@goetas
Copy link
Member

goetas commented Jun 21, 2020

thanks @guilliamxavier

I think that I will have fun times when merging back this to master :)

@goetas goetas added this to the 2.3.0 milestone Jun 21, 2020
@guilliamxavier guilliamxavier deleted the patch-1 branch June 22, 2020 08:08
goetas added a commit that referenced this pull request Nov 22, 2020
- Total issues resolved: **0**
- Total pull requests resolved: **6**
- Total contributors: **6**

 - [1032: Let composer decide the best version](#1032) thanks to @PowerKiKi

 - [1020: Add badges into README about license and packagist](#1020) thanks to @matks
 - [999: Allow using on PHP 7.1 with Composer 2](#999) thanks to @nicolas-grekas

 - [981: Add "from-empty-schema" option for "diff" command](#981) thanks to @guilliamxavier
 - [954: Make compared tables order idempotent](#954) thanks to @julienfalque

 - [888: Use executeUpdate instead of executeQuery for write operation](#888) thanks to @goetas
goetas added a commit that referenced this pull request Nov 22, 2020
- Total issues resolved: **0**
- Total pull requests resolved: **6**
- Total contributors: **6**

 - [1032: Let composer decide the best version](#1032) thanks to @PowerKiKi

 - [1020: Add badges into README about license and packagist](#1020) thanks to @matks
 - [999: Allow using on PHP 7.1 with Composer 2](#999) thanks to @nicolas-grekas

 - [981: Add "from-empty-schema" option for "diff" command](#981) thanks to @guilliamxavier
 - [954: Make compared tables order idempotent](#954) thanks to @julienfalque

 - [888: Use executeUpdate instead of executeQuery for write operation](#888) thanks to @goetas
goetas added a commit that referenced this pull request Nov 22, 2020
- Total issues resolved: **0**
- Total pull requests resolved: **6**
- Total contributors: **6**

 - [1032: Let composer decide the best version](#1032) thanks to @PowerKiKi

 - [1020: Add badges into README about license and packagist](#1020) thanks to @matks
 - [999: Allow using on PHP 7.1 with Composer 2](#999) thanks to @nicolas-grekas

 - [981: Add "from-empty-schema" option for "diff" command](#981) thanks to @guilliamxavier
 - [954: Make compared tables order idempotent](#954) thanks to @julienfalque

 - [888: Use executeUpdate instead of executeQuery for write operation](#888) thanks to @goetas
@alzee
Copy link

alzee commented May 17, 2021

Thanks @guilliamxavier
Great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants