Table Command to KeyValue store Mapping#5
Open
darshannevgi wants to merge 10 commits intoacharapko:masterfrom
Open
Table Command to KeyValue store Mapping#5darshannevgi wants to merge 10 commits intoacharapko:masterfrom
darshannevgi wants to merge 10 commits intoacharapko:masterfrom
Conversation
darshannevgi
force-pushed
the
TableToKV
branch
2 times, most recently
from
df6bb6e to
aab0adb
Compare
darshannevgi
force-pushed
the
TableToKV
branch
from
aab0adb to
454c449
Compare
acharapko
reviewed
Dec 1, 2019
tablestore/db_test.go
Outdated
| "fmt" | ||
| ) | ||
|
|
||
| func TestSave(t *testing.T) { |
Owner
There was a problem hiding this comment.
Make sure to actually assert your tests for expected output, not just print to console
acharapko
reviewed
Dec 1, 2019
| // "fmt" | ||
| ) | ||
|
|
||
| func TestTranslateToKV(t *testing.T) { |
Owner
There was a problem hiding this comment.
Make sure tests actually test different scenarios for correctness. Why are all assertions commented out?
acharapko
reviewed
Dec 1, 2019
tablestore/utility.go
Outdated
| } | ||
|
|
||
| //Incomplete Implementation | ||
| func parseKey(data []KVItem){ |
Owner
There was a problem hiding this comment.
Make sure to add a test to this function
Owner
There was a problem hiding this comment.
Also, try to test every major function you are writing
Owner
There was a problem hiding this comment.
and of course, if this is all commented out, just remove.
acharapko
reviewed
Dec 1, 2019
tablestore/database.go
Outdated
| batch.Put(item.Key,item.Value) | ||
| } | ||
| } | ||
| //batch.Put([]byte("foo"), []byte("value")) |
Owner
There was a problem hiding this comment.
clean up the commented code. If something needs to be commented, please describe it in English and use example code if needed. But do not leave just lines and lines of commented code
acharapko
reviewed
Dec 1, 2019
|
|
||
| } | ||
|
|
||
| func (db *database) Save(tableid uuid.UUID, items []KVItem) error { |
acharapko
reviewed
Dec 1, 2019
tablestore/database.go
Outdated
| US || NY || Buffalo || University Avenue | ||
| US || NY || Columbus || EngleWood Avenue | ||
| */ | ||
| func (db *database) GetPartition(tableid uuid.UUID, partitionKeys []FleetDBValue , columns []string, tableSpec FleetDBTableSpec) ([]Row, error) { |
acharapko
reviewed
Dec 1, 2019
tablestore/database.go
Outdated
| return rows,err | ||
| } | ||
|
|
||
| func createRow(partitionKeys []FleetDBValue, oldCKeyvalues []FleetDBValue, oldColvalues []FleetDBValue) Row{ |
acharapko
reviewed
Dec 1, 2019
| return Row{row} | ||
| } | ||
|
|
||
| func createRowFromMap(valMap map[string]FleetDBValue, columns []string) Row{ |
acharapko
reviewed
Dec 1, 2019
| return Row{row} | ||
| } | ||
|
|
||
| func equal(val1 []FleetDBValue, val2 []FleetDBValue) bool{ |
darshannevgi
force-pushed
the
TableToKV
branch
6 times, most recently
from
bdaecd0 to
e644d01
Compare
darshannevgi
force-pushed
the
TableToKV
branch
from
e644d01 to
996f39e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Features:
4)Test files for testing all added features