enpassio / Databinding

Various examples on databinding which can help not only understand how to get started with it but it also shows how this can be used in complex and real use cases
Apache License 2.0
26 stars 14 forks source link

Dev databinding newsapi kotlin #43

Closed rajtheinnovator closed 5 years ago

rajtheinnovator commented 5 years ago

@OyaCanli can you please look into this branch. As we were fixing the merge conflict by copying the files directly so I guess, we'd need to do the same thing again. Please go through it when you get the time and let's discuss as and when need be. Thanks :+1:

OyaCanli commented 5 years ago

@rajtheinnovator I don't understand. The previous case was different. Greta had converted the existing java project to kotlin. Here, I worked on a brand new project. I copy pasted some resources, which might be a mistake. But I didn't copy paste gradle files or manifest.. I'll try to find out the reason of conflict..

EDIT: In fact I copy pasted some resources between two-way databinding java and kotlin versions as well and that didn't create a conflict. There should be something else. I'll look at that tomorrow with a clear mind.

EDIT2 : Aha.. I hadn't created a brand new project in fact. You had pushed initial empty projects to all initial four branches. So I worked on the project that you had put to newsapi-kotlin branch. But now I see in the commit history that you did something similar to what Greta did. You converted the java version to kotlin. You should have put a brand new project. Anyway.. So we face the same kind of conflict again. I'll think of a solution tomorrow.

rajtheinnovator commented 5 years ago

@OyaCanli you're right. It seems to be caused because I made a mistake while initial project setup. Looks like I first created Java version and changed the same to Kotlin one. Happened probably because I was setting up projects in multiple branches simultaneously. As it'd be best suited if we've bug free separate branches so as per your suggestion, let me delete the existing kotlin branch for this sample and re-create it from initial commit. You can then make a new PR in that branch.

rajtheinnovator commented 5 years ago

@OyaCanli seems like I was able to handle the merge conflict. So it should be fine for now and you can work on the articles. If later on we get another conflict, we can try fixing that again or recreate this branch from scratch as discussed above.

OyaCanli commented 5 years ago

@rajtheinnovator Nope, it doesn't seem fine to me. You shouldn't have merged this one. And I should have closed this PR as soon as I made the other. Now both test1 branch and newsapi-kotlin branch seem to be messed up.

rajtheinnovator commented 5 years ago

@OyaCanli when I tried creating another PR from your forked branch, it seemed fine to me and hence I went with the merge but as you say it's giving issue, I can revert back the commits I made. Should I try doing that? Or maybe you can make a PR to dev-databinding-newsapi-kotlin branch from your side first and then we can see how it goes. Can you also share in what branch it's giving errors?

OyaCanli commented 5 years ago

Oh I see this now @rajtheinnovator To resolve the issue we had yesterday, I had created a new branch for replacing newsapi kotlin locally and once I'm done, I tried to merge it to test1(locally again). And since it was successfull, I directly pushed test1 and made a PR. Once you accepted that PR, you were done on test1 branch. Accepting yesterday's conflicting PR on it, messed it up. NewApi java version was lacking some of the components in test1 branch and I see that you also merged test1 into newsapi kotlin. Anyway.. No worries, it happens. Reverting was the right thing to do. It seems fine to me now. When you have occation to test test1 branch, if everything is ok, you can merge it master.

For the newsapi-kotlin branch, when you have time, you can erase that one. Then checkout to the initial commit and create a new branch from that point. I'll then make a PR to that branch, just to keep it synched..

rajtheinnovator commented 5 years ago

@OyaCanli yeah, I noticed it after you said you're seeing some issues after I merged. Actually while merging, I didn't notice that I was merging again to test1 rather than in databinding-newswapi-kotlin branch. Thanks for staying here all the time :+1:

Yeah, I'll manually check all the samples once by running them on my device and after that, I'll merge them to master. Finally, as you said, I'll recreate branch from scratch so you can make another PR, by next tomorrow :+1: