These are chat archives for dgraph-io/dgraph

3rd
Feb 2016
Manish R Jain
@manishrjain
Feb 03 2016 04:13
Hey @ashwin95r, the pull request looks great. One last comment, and it's good to submit.
Ashwin Ramesh
@ashwin95r
Feb 03 2016 05:19
Great, Ill finish that in sometime and run it in docker
Manish R Jain
@manishrjain
Feb 03 2016 05:32
Sounds great!
https://reviewable.io/ works for me.
oh.. just gave me an error as well.
Ashwin Ramesh
@ashwin95r
Feb 03 2016 05:33
Ah, Okay
Manish R Jain
@manishrjain
Feb 03 2016 05:33
Seems like reviewable is based on Firebase.
How do you like it so far?
Ashwin Ramesh
@ashwin95r
Feb 03 2016 05:35
It has been good. Got a hang of the pull requests and the code reviews
Manish R Jain
@manishrjain
Feb 03 2016 05:35
Yeah, you did a good job with doing the updates and sync-ing them.
Maybe if you get a chance, write a small wiki page about how to do this; your own experience + some useful references -- I'm sure many others could use that information.
Ashwin Ramesh
@ashwin95r
Feb 03 2016 05:36
Even if i acknowledge a comment in reviewable, it still remains unresolved
is there something I'm missing?
Manish R Jain
@manishrjain
Feb 03 2016 05:37
I think only a couple of them should act like that -- the ones I mark explicitly as blocking
Those only resolve if I mark them as resolved.
Ashwin Ramesh
@ashwin95r
Feb 03 2016 05:38
Okay, so I made the last change that you had mentioned
now should I create a new branch and continue working?
Manish R Jain
@manishrjain
Feb 03 2016 05:39
If you made the last change, you can go ahead and submit.
You have the permissions to do so, as one of the maintainers.
Ashwin Ramesh
@ashwin95r
Feb 03 2016 05:39
I synced it in my repository
the pull request has to be closed now?
Manish R Jain
@manishrjain
Feb 03 2016 05:40
You have to merge the pull request.
There's a button in there.
dgraph-io/dgraph#16
Ashwin Ramesh
@ashwin95r
Feb 03 2016 05:41
Should I first run it before doing that?
Manish R Jain
@manishrjain
Feb 03 2016 05:41
If you merge, pull request would automatically close.
You don't need to close it.
Ashwin Ramesh
@ashwin95r
Feb 03 2016 05:42
Okay
Manish R Jain
@manishrjain
Feb 03 2016 06:22
Hey @ashwin95r, would be good to have some unit tests, which can call the uid assigner function, with numInstances = 2, and instanceIdx = [0, 1]; and compare that against numInstances = 1, and instanceIdx = 0
That should be sufficient for now. We can do integration testing for v0.2 close to time of release.
Btw, #16 hasn't yet been merged. Are you waiting on something?
Ashwin Ramesh
@ashwin95r
Feb 03 2016 06:23
Okay Ill read up on testing in Go, We can discuss about it during tomorrows meeting
but shouldnt it be tested before merging?
Manish R Jain
@manishrjain
Feb 03 2016 06:24
Yeah, unit tests would be ideal for this.
There're plenty of unit test examples in the code base. Testing in Go is really easy.
If you haven't written tests yet, I'd suggest merging this one, if it doesn't break the build. And sending another pull request with the right tests.
No point in blocking this one.
Ashwin Ramesh
@ashwin95r
Feb 03 2016 06:26
Okay Ill merge it now
Joao Martins
@jcmartins
Feb 03 2016 12:30
how can I model this http://bit.ly/1mcLkfO to dgraphdb?
Manish R Jain
@manishrjain
Feb 03 2016 22:24
Will look at it in a couple of hours, and let you know.
Manish R Jain
@manishrjain
Feb 03 2016 23:39
So, the setup here looks pretty flat -- something that a relational db can do best.
But, surely you can set this up in graph db. I think ship delivery would be a node, which would have properties like Name, street, state, etc.
Then order is a node, creditcard is a node. Not unlike how you have set it up right now.
Looks like a good setup to me.
@jcmartins ^