-
-
Notifications
You must be signed in to change notification settings - Fork 259
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
Failing one to one bidirectional fix #218
Failing one to one bidirectional fix #218
Conversation
@@ -1,3 +1,4 @@ | |||
.idea/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be part of your global gitignore, not of a particular project. Since it is specific to your local setup, not to this project. (related article https://thomashunter.name/blog/global-gitignore-vs-repository-gitignore/)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tolry We're actually versioning the .idea
folder in order to share IntelliJ settings across developers (https://intellij-support.jetbrains.com/hc/en-us/articles/206544839-How-to-manage-projects-under-Version-Control-Systems) so I can't have it in the global gitignore. However, I will remove the .gitignore
change from this PR.
@tolry @bendavies Any updates on this? I think this bug is quite severe, as it basically breaks the bundle functionality (returning wrong data) for certain types of relations. /cc @jfierz |
Hi Peter, Sorry for the delay. I'm away on holiday right now so can't take a close look. We'll get this merged when I return! Thanks |
No worries ben, thanks for the quick response. Enjoy your holiday ⛱ |
@bendavies Would you mind taking a look on this one? Seems like a critical issue since the data returned by the bundle is wrong. |
LGTM Can you resolve the conflicts? Then I would merge it. |
ack, sorry i totally forgot about this. yes looks fine once conflicts are resolved! |
73fedea
to
addd825
Compare
Rebased on the latest |
The tests are failed. I have restarted the travis build for these PR and Master. |
postgres builds are broken (also in master), it hang out and i don't know why :-/ |
Hm, but it seems to be an issue on the master as well, yeah. |
I will merge this because the tests are running before you rebased (the conflict was only in RelationTest.php). |
btw. I have created a issue for the postgres builds: #236 |
Great, thanks! Will you tag a new version any time soon? |
i will create a new version if we fixed the postgres builds ;-) |
@peschee thanks for contributing! |
PR for #217