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.

Sign up for GitHub

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

Jump to bottom

Proposal changes to the Mapped Repositories Interfaces #13

Closed
Lctrs opened this issue Mar 24, 2017 · 3 comments · May be fixed by #17
Closed

Proposal changes to the Mapped Repositories Interfaces #13

Lctrs opened this issue Mar 24, 2017 · 3 comments · May be fixed by #17

Comments

@Lctrs
Copy link
Contributor

Lctrs commented Mar 24, 2017

Currently, their names can be confusing with our "real" repositories interfaces defined in php-xapi/repository-api (by real, I mean the ones that we'll use in the LrsBundle).

So, I propose to mark them as internal and change their suffix part to Storage (ex. : StatementStorage for XApi\Repository\Doctrine\Repository\Mapping\StatementRepository).

WDYT @xabbuh ?
@xabbuh
Copy link
Contributor

xabbuh commented Mar 24, 2017

But in terms of Doctrine these classes are indeed repositories. So maybe we could remove the Mapping namespace and change the class name to XApi\Repository\Doctrine\Repository\MappingStatementRepository. What do you think?

@Lctrs
Copy link
Contributor Author

Lctrs commented Mar 27, 2017

The implementations are, our interfaces are not (even if they are only meant to be implemented by a Doctrine Repository).

I don't find MappingStatementRepository to be very explicit. :/

@xabbuh
Copy link
Contributor

xabbuh commented Mar 30, 2017

The implementations are, our interfaces are not (even if they are only meant to be implemented by a Doctrine Repository).

Fair point. :) Please take a look at #17.

@xabbuh xabbuh mentioned this issue Mar 30, 2017
Open
@Lctrs Lctrs closed this as completed Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rename internal repository to StatementStorage xabbuh/repository-doctrine
2 participants
@xabbuh @Lctrs