support for dynamic tablenames for multi-tenancy #318 #319
Conversation
|
I see that the test coverage % has not passed. I can fix that. Please let me know if you have any other feedback. Thanks, Biju |
|
I added some more test code to increase test coverage. When I build on my dev machine I see 71% test coverage. Coveralls is reporting 60.9% test coverage.. I am not sure which one to believe.. :-) PASS |
…refactored test code to cover more APIs with dynamic table
gorp.go
Outdated
| // use different database table names during runtime | ||
| // while sharing the same golang struct for in-memory data | ||
| type DynamicTable interface { | ||
| GetTableName() string |
There was a problem hiding this comment.
In keeping with the getters section of Effective Go, can we rename this to TableName()?
There was a problem hiding this comment.
Sure, I can change it to TableName(), will update the code within the next couple of days. Thanks!
Biju
|
Sorry about the long delay; I'm going to start going through pull requests gradually and hopefully get things moving toward v2 again. The only thing I saw that I would prefer changed is the |
|
Weird style for "if" checks. It's the opposite of how standard Go and Gorp uses them, except when you check errors, then it's standard. |
|
@stxml : good catch 👍 It is an old habit of mine from C++ days (constants on the left of equality operator to avoid accidental assignment). |
|
I'll just swap the order post-merge :) I'm going over this one last time and am going to run tests locally. |
| } | ||
| if nil == dbObjs { | ||
| t.Errorf("Nil return from dbmap.Select") | ||
| } else { |
There was a problem hiding this comment.
I'm also updating this - t.Fatalf will perform the same comparison but halt the test immediately, making this else statement unnecessary.
|
Thank you very much @nelsam ! |
This is a pull request to implement dynamic table names as described in issue #318