doctrine / data-fixtures

Doctrine2 ORM Data Fixtures Extensions
http://www.doctrine-project.org
MIT License
2.77k stars 224 forks source link

Fix doctrine/data-fixtures PR#334 #340

Closed stephen-lewis closed 4 years ago

stephen-lewis commented 4 years ago

PR #334 broke existing code by adding quotes in the middle of table names that used schemas in SQLite - as reported in #338 .

I've reworked it into a cleaner function that mimics SchemaTool in doctrine/orm. Tests included and pass locally; I've also setup and run a simple test project locally using schema and non-schema entities without issue.

greg0ire commented 4 years ago

@stephen-lewis can you please run this script from your fork? It should produce introduce for people who need to test.

stephen-lewis commented 4 years ago

@stephen-lewis can you please run this script from your fork? It should produce introduce for people who need to test.

Sure; output below:

How can I test this?

composer config repositories.stephen-lewis vcs https://github.com/stephen-lewis/data-fixtures
composer require doctrine/data-fixtures "dev-bugfix/fix_errors_from_PR334 as 1.4.1"

--edit: fix shell command typo

greg0ire commented 4 years ago

@jgxvx please use the instructions above to test this on your project and report back :pray: You might have to use the no-api key, but I'm not 100% sure about that, please see a usage example here: https://github.com/composer/composer/issues/5433

greg0ire commented 4 years ago

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/1.4.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.
jgxvx commented 4 years ago

@greg0ire @stephen-lewis Glad to confirm that the fix works!

greg0ire commented 4 years ago

@stephen-lewis please improve your commit message according to the contributing guide, then I will merge

TL;DR say what was broken and how your changes improve the situation

stephen-lewis commented 4 years ago

@greg0ire commit message should now be updated :smile: