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

Expose the connection in DependencyFactory #893

Merged
merged 1 commit into from
Dec 15, 2019

Conversation

goetas
Copy link
Member

@goetas goetas commented Dec 14, 2019

Q A
Type feature
BC Break no-

Summary

Expose the connection from the DependencyFactory to ease the testing of the bundle

@greg0ire
Copy link
Member

greg0ire commented Dec 14, 2019

Is there a reason not to contribute this on the stable branch?

@goetas
Copy link
Member Author

goetas commented Dec 15, 2019

Mainly because I needed it to write the tests for the new bundle, that will support only migrations 3.x.

@greg0ire
Copy link
Member

I'd rather have the branches diverge as little as possible, because otherwise merges up will become more and more painful. I had a quick look, and merging up comes with only one conflict, so I think it could be done quickly after this is merged in the stable branch. I'd like to know what @alcaeus thinks about this (the last merge up into master was #867, but it was with 2.1.x, not 2.2.x or 2.3.x, I'm not sure why). If it turns out to be too complicated we should merge into master directly.

@goetas
Copy link
Member Author

goetas commented Dec 15, 2019

I had a quick look, and merging up comes with only one conflict, so I think it could be done quickly after this is merged in the stable branch. I'd like to know what @alcaeus thinks about this (the last merge up into master was #867, but it was with 2.1.x, not 2.2.x or 2.3.x, I'm not sure why). If it turns out to be too complicated we should merge into master directly.

I was talking with @alcaeus about that and he has to show me how the merging is done with the release tool.
On the other side, I think that sooner or later 2.x and 3.x will diverge, and backporting commits will be pointless since 2.x and 3.x are really different codebases now. I can continue the backport for some time.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Anyway, the code looks good, so do as you think is best.

@goetas goetas merged commit e064cec into doctrine:master Dec 15, 2019
@goetas goetas deleted the connection branch December 15, 2019 15:15
@goetas
Copy link
Member Author

goetas commented Dec 15, 2019

Thanks! I'm merging it as it is. Solving conflicts will be a pain anyway, but I do not think that will happen on this lines of code in the DependencyFactory class.

@goetas goetas added this to the 3.0.0 milestone Mar 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants