-
Notifications
You must be signed in to change notification settings - Fork 157
[WIP] Feature/doctrine #497
base: master
Are you sure you want to change the base?
Conversation
What do you think, can I help with a PR that prepares for this and adds the capability to run Doctrine migrations? |
} | ||
|
||
public function indexAction() | ||
{ | ||
$query = $this->params()->fromQuery('query', null); | ||
|
||
$results = $this->moduleMapper->findByLike($query); | ||
$results = $this->moduleService->findModules($query, ['m.name']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@localheinz not sure about this but is it legal to place this param with his specific prefix m.name
in the controller?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably better not to expose internals to much - so how about (if need be) exposing a method which fill find modules by name, specifically (if that's what it does)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's what i thought too, i will change this, ty
Yes please! I'm not very familiar with Doctrine Migrations nor how to setup this feature |
Doctrine migrations are implemented in #502. About this PR, what do you think about breaking it up into smaller parts? Since we agreed that we want to use Doctrine, it doesn't need to be implemented all in one PR. Maybe start with creating entities and making sure we've got the schema right? Then, in a next step, start using them. We should be running
as part of the What do you think? |
@localheinz sound good to me. what do u suggest, changing the current db structure - creating doctrine entitys or vice versa? This is the first time i meet doctrine migrations (oh boy - don't let @Ocramius hear that 😸 ) so currently i don't know for sure what had changed, if someone can give me a short hint with this that would be nice. /cc @gianarb |
IMO we can use doctrine\migrations to change the current database schema. A migration is very easy to implement. We can write a SQL script to update our struct.. And another SQL script to down it in the old version |
Depends entirely on which way you want to go - do you have a preference? By the way, |
In regard to the migrations, as @gianarb suggested, you can start by creating migrations as suggested: $ php public/index.php migrations:generate This will create a migration class, which you can modify to your needs. If you have a migration which you can't or don't ever want to be able to roll back, you should probably throw a |
thanks @gianarb @localheinz , so i think i will stop here and handle the remaining todos (described above) in different prs as this is more clear, do u agree? the proposed db change #496 don't affect this improvement nor needs db changes so i think migration changes will not occur in this and the following prs |
Yes.. It sounds good :) More easy to merge and to follow :) 2015-10-19 11:11 GMT+02:00 Marco Rieger notifications@github.com:
Gianluca Arbezzano |
cffd688
to
482ff43
Compare
This PR is adding Doctrine ORM to the Project and will replace the Zend\Db Mappers.
Follows after Discussion in #496
@localheinz @gianarb @Ocramius @GeeH