Make getUid global

[ashwin95r]

[CLAassistant]

All committers have accepted the CLA.

[manishrjain]

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

---

loader/loader.go, line 154 [r1] (raw file):
... % s.mod shouldn't always be zero. It should be the index set for the server.

---

loader/loader.go, line 158 [r1] (raw file):
No need for else after a continue.

---

loader/loader.go, line 163 [r1] (raw file):
Add a log here as well.

---

loader/loader.go, line 173 [r1] (raw file):
if len(nq.ObjectId) == 0 || farm.Fingerprint64([]byte(nq.ObjectId)) % s.mod != s.idx {

---

loader/loader.go, line 177 [r1] (raw file):
No need for else again.

---

Comments from the review on Reviewable.io

[manishrjain]

Review status: 0 of 6 files reviewed at latest revision, 10 unresolved discussions.

---

loader/loader.go, line 158 [r1] (raw file):
So, continue is great for code readability. It avoids having to scan the rest of the code in the block to see if there's anything more. You should get rid of else instead; unless there's a particular reason you can't.

---

loader/loader.go, line 149 [r3] (raw file):
You got it wrong here, but I can see why... We should change the names of the variables to numInstances and instanceIdx.

---

loader/loader.go, line 169 [r3] (raw file):
Same problem here.

---

query/query.go, line 258 [r3] (raw file):
In a % b == c, mod generally refers to b, and c to index.

---

server/uidassigner/main.go, line 22 [r3] (raw file):
Call it instanceIdx, both in var and in the flag.

---

server/uidassigner/main.go, line 23 [r3] (raw file):
Call it numInstances

---

Comments from the review on Reviewable.io

[ashwin95r]

Review status: 0 of 6 files reviewed at latest revision, 10 unresolved discussions.

---

loader/loader.go, line 149 [r3] (raw file):
Actually I named the machine index as mod and the total number of instances as numInstance. Maybe I should rename them to clear the confusion

---

Comments from the review on Reviewable.io

[manishrjain]

loader/loader.go, line 149 [r3] (raw file):
Yeah, do that.

---

Comments from the review on Reviewable.io

[ashwin95r]

Review status: 0 of 8 files reviewed at latest revision, 10 unresolved discussions.

---

loader/loader.go, line 158 [r1] (raw file):
Here, since we have to check if the subject and object can be assigned UID, we need two if statements and continue here would render the following if useless which isn't correct

---

Comments from the review on Reviewable.io

[manishrjain]

Reviewed 5 of 6 files at r2, 1 of 1 files at r3.
Review status: 2 of 8 files reviewed at latest revision, 10 unresolved discussions.

---

loader/loader.go, line 158 [r1] (raw file):
This looks like a bit of repeated code between allocating for subject and objectid. Move it out into a function and reuse it.

---

loader/loader.go, line 50 [r4] (raw file):
numInstances -- plural; as I mentioned below as well.

---

query/query.go, line 258 [r4] (raw file):
Change the comment to use the updated variable names.

---

Comments from the review on Reviewable.io

[manishrjain]

One last small comment. But, otherwise ! Good job!

---

Reviewed 6 of 6 files at r4, 6 of 6 files at r5.
Review status: all files reviewed at latest revision, 9 unresolved discussions.

---

uid/assigner.go, line 97 [r5] (raw file):
Do mod first, and then use it here.
minIdx := instanceIdx * mod

---

Comments from the review on Reviewable.io