Callerid2

[dumbunny]

@guokeno0 Here is a small change that adds some more test coverage for effective_caller_id.

[michael-berlin]

@dumbunny A quick note on the PR itself: You pushed the branch to the official Github repository and not your fork dumbunny/vitess.

That's fine for now. I'll revise our Git + Github documentation tomorrow. Once that's available, let's make sure that you follow the triangular workflow using your fork then :)

[alainjobart]

This 0x80 is a bit confusing, I think you meant 0x8000000000 instead?

0x80 would be one of the first values in the 00-80 range for instance, not the middle value as one would expect when you see 0x80 like this...

[enisoc]

Alternatively, you could use struct.Struct('Q').pack(0x80), but I guess that might be even more confusing.

[guokeno0]

@dumbunny I also talked with Liang yesterday and looks like you guys are all using branches from the youtube/vitess master yet I'm using my fork and sync with the upstream every time. That's why your push will always go to the master directly

[yaoshengzhe]

@dumbunny

git checkout callerid2
// make change locally
git add .
git commit -amend
git push -f // this forces changes in github in sync with your local change
// at this point, your PR should reflect your latest change

[yaoshengzhe]

FYI, we prefer to use forked model to do development. Fork youtube/vitess repo in your own github repo, do the development, send PR to youtube/vitess for your changes :)

[michael-berlin]

@guokeno0 @yaoshengzhe

I'll update our Git instructions later to reduce the confusion around this and avoid accidental pushes to the official Github repo in the future.

[guokeno0]

I think this test should cover all possible cursor APIs in vtgate_cursor.py so that we will be confident that we don't miss any piece of callerid on the server side

[guokeno0]

Overall LGTM, it is better to improve the test coverage by testing other cursor APIs