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

split annotations into separate files #412

Merged
merged 3 commits into from
Feb 4, 2014
Merged

split annotations into separate files #412

merged 3 commits into from
Feb 4, 2014

Conversation

gnat42
Copy link
Contributor

@gnat42 gnat42 commented Feb 3, 2014

Allow the phpcr annotations to be used without modifying the app/autoload.php
file. The DoctrineAnnotations does a require_once to ensure existing projects
can continue to function

Allow the phpcr annotations to be used without modifying the app/autoload.php
file. The DoctrineAnnotations does a require_once to ensure existing projects
can continue to function
* @Target("METHOD")
*/
final class PostLoad {}
require_once('./Document.php');
Copy link
Member

Choose a reason for hiding this comment

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

think its necessary to use __DIR__ here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so like?

require_once( __DIR__ .'/Document.php');

Copy link
Member

Choose a reason for hiding this comment

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

yes. though I would prefer require_once __DIR__ .'/Document.php';

@dantleech
Copy link
Contributor

Looks good, I know the things I point out were there in the original code, but seems like a good time to fix them.

*/
public $type = 'name';
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Empty line at end of file

@@ -21,355 +21,41 @@

use Doctrine\Common\Annotations\Annotation;
Copy link
Member

Choose a reason for hiding this comment

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

this use statement can be removed. i propose that we add a comment in this file that its not needed to include this anymore as autoloading works. something like

/** 
 * Deprecated
 *
 * Including this file is no longer needed, as each annotation was moved into its own file
 * supporting PSR-0 compatible autoloading.
 * The file is kept for backwards compatibility and will be removed in a future version of phpcr-odm.
 */

Copy link
Member

Choose a reason for hiding this comment

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

added that command directly in master. thanks again @gnat42

@dbu
Copy link
Member

dbu commented Feb 4, 2014

awesome! travis passing means there are most likely no mistakes in this. did you test this in a project too, or only with the tests?

lsmith77 added a commit that referenced this pull request Feb 4, 2014
split annotations into separate files
@lsmith77 lsmith77 merged commit b480ca4 into doctrine:master Feb 4, 2014
dbu added a commit that referenced this pull request Feb 4, 2014
inssein added a commit to inssein/symfony-cmf-docs that referenced this pull request Jul 17, 2015
Registering annotations should no longer be necessary. (doctrine/phpcr-odm#412)
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.

4 participants