Skip to content
This repository has been archived by the owner on May 1, 2019. It is now read-only.

[WIP] Feature/doctrine #497

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

ins0
Copy link
Contributor

@ins0 ins0 commented Sep 19, 2015

This PR is adding Doctrine ORM to the Project and will replace the Zend\Db Mappers.
Follows after Discussion in #496

  • added doctrine
  • added module entity
  • added user entity
  • changed application module
  • changed user module
  • changed zfmodule module
  • remove old mapper
  • edit existing tests

@localheinz @gianarb @Ocramius @GeeH

@localheinz
Copy link
Member

@ins0

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']);
Copy link
Contributor Author

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?

Copy link
Member

Choose a reason for hiding this comment

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

@ins0

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)?

Copy link
Contributor Author

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

@ins0
Copy link
Contributor Author

ins0 commented Sep 23, 2015

@localheinz

What do you think, can I help with a PR that prepares for this and adds the capability to run Doctrine migrations?

Yes please! I'm not very familiar with Doctrine Migrations nor how to setup this feature

@localheinz
Copy link
Member

@ins0

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

$ php public/index.php orm:info
$ php public/index.php orm:validate-schema

as part of the script on Travis, and just make sure for a start that the mappings are right.

What do you think?

@ins0
Copy link
Contributor Author

ins0 commented Oct 19, 2015

@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

@gianarb
Copy link
Contributor

gianarb commented Oct 19, 2015

IMO we can use doctrine\migrations to change the current database schema.
After this process we can write a correct entities.. :)

A migration is very easy to implement.
http://docs.doctrine-project.org/projects/doctrine-migrations/en/latest/reference/migration_classes.html.

We can write a SQL script to update our struct.. And another SQL script to down it in the old version

@localheinz
Copy link
Member

@ins0

Depends entirely on which way you want to go - do you have a preference?

By the way, zendframework/modules.zendframework.com:1.6.0 has been deployed - with a little downtime, as I forgot to edit the local configuration files before deploying, but, it's up!

@localheinz
Copy link
Member

@ins0

Following the merge of #502, you need to rebase, sorry!

@localheinz
Copy link
Member

@ins0

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 Doctrine\DBAL\Migrations\IrreversibleMigrationException.

@ins0
Copy link
Contributor Author

ins0 commented Oct 19, 2015

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

@gianarb
Copy link
Contributor

gianarb commented Oct 19, 2015

Yes.. It sounds good :)

More easy to merge and to follow :)

2015-10-19 11:11 GMT+02:00 Marco Rieger notifications@github.com:

thanks @gianarb https://github.com/gianarb @localheinz
https://github.com/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?


Reply to this email directly or view it on GitHub
#497 (comment)
.

Gianluca Arbezzano
www.gianarb.it

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

Successfully merging this pull request may close these issues.

4 participants