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

ACL + DoctrineMigrationsBundle #2091

Closed
rubensayshi opened this issue Sep 5, 2011 · 17 comments · Fixed by #3560
Closed

ACL + DoctrineMigrationsBundle #2091

rubensayshi opened this issue Sep 5, 2011 · 17 comments · Fixed by #3560
Labels

Comments

@rubensayshi
Copy link

The app/console doctrine:migrations:diff generates a migration to drop all the ACL tables.

Now I'm pretty sure we can safely say that a develop should always check the generated migrations ...
Nonetheless this behavory can be anoying and is defenatly not intended!

PS . I'm not really sure where this issue should go but I think this is more related to ACL then the bundle.
Apart from that I think the ACL has some serious issues with being decoupled while instead it should be strongly integerated into the doctrine bundle.

@acasademont
Copy link
Contributor

+1 for this, i made a comment on the Google Groups list a couple of months ago, let's see if i can do something in the code

@lsmith77
Copy link
Contributor

@schmittjoh ping

@schmittjoh
Copy link
Contributor

I had submitted a patch to solve this which was rejected. It's somewhere in my symfony fork though.

@vicb
Copy link
Contributor

vicb commented Mar 8, 2012

@schmittjoh could you give the ref to your PR / branch ?

@schmittjoh
Copy link
Contributor

Unfortunately not, it's incorporated into my master branch. But since it
already was rejected, I guess we can close it.

On Thu, Mar 8, 2012 at 2:36 PM, Victor Berchet <
reply@reply.github.com

wrote:

@schmittjoh could you give the ref to your PR / branch ?


Reply to this email directly or view it on GitHub:
#2091 (comment)

@vicb
Copy link
Contributor

vicb commented Mar 8, 2012

Do you remember the reason why this was rejected by any chance ? (I can't find the closed PR with GH)

@schmittjoh
Copy link
Contributor

I believe it was performance, but don't know for sure, was like 7 or 8
months ago.

On Thu, Mar 8, 2012 at 3:15 PM, Victor Berchet <
reply@reply.github.com

wrote:

Do you remember the reason why this was rejected by any chance ? (I can't
find the closed PR with GH)


Reply to this email directly or view it on GitHub:
#2091 (comment)

@vicb
Copy link
Contributor

vicb commented Mar 8, 2012

#1313 ?

@schmittjoh
Copy link
Contributor

Yes, that looks like it.

@vicb
Copy link
Contributor

vicb commented Mar 9, 2012

So I guess the problem was mainly that the PR has 2 not related commits (am I right here ?) + some mis-understanding. Would you mind opening a new PR with the second commit only ?

@schmittjoh
Copy link
Contributor

I'd appreciate if someone else can cherry-pick the commit, and try to get
it in. I don't have that much time unfortunately.

On Fri, Mar 9, 2012 at 3:18 AM, Victor Berchet <
reply@reply.github.com

wrote:

So I guess the problem was mainly that the PR has 2 not related commits
(am I right here ?) + some mis-understanding. Would you mind opening a new
PR with the second commit only ?


Reply to this email directly or view it on GitHub:
#2091 (comment)

@vicb
Copy link
Contributor

vicb commented Mar 9, 2012

I can do that if you confirm that the first commit is not required.

@memphys
Copy link
Contributor

memphys commented Aug 25, 2012

Was the problem really fixed? I still have the issue in version 2.0.16. Any ideas on how to overcome this?

@stof
Copy link
Member

stof commented Aug 25, 2012

@memphys it was fixed, but for 2.1

@memphys
Copy link
Contributor

memphys commented Aug 26, 2012

@stof any chances it will be back-ported to 2.0?

@stof
Copy link
Member

stof commented Aug 26, 2012

@memphys no. It relies on a new feature introduced in 2.1, and new features are never backported.

@memphys
Copy link
Contributor

memphys commented Aug 26, 2012

@stof ok, understood. Thanks!

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

Successfully merging a pull request may close this issue.

8 participants