From b92551d711474f11954435f8adcc60bc86dcbdc0 Mon Sep 17 00:00:00 2001 From: Aaron Boodman Date: Thu, 18 Jun 2015 18:11:52 -0700 Subject: [PATCH] Move the TODOs into bugs ... it is easier that way to refer to them from within the code --- TODO.md | 11 ----------- chunks/memory_store.go | 1 + user/user.go | 2 +- 3 files changed, 2 insertions(+), 12 deletions(-) delete mode 100644 TODO.md diff --git a/TODO.md b/TODO.md deleted file mode 100644 index 90579a0977..0000000000 --- a/TODO.md +++ /dev/null @@ -1,11 +0,0 @@ -Really bad: -* We eagerly read refs recursively, which for many parts of the system results in ridiculous inefficency (e.g., inside chunks.Commit()) -* Using the Value types is very frustrating because of all the wrapping and unwrapping Go types in Values. We need to either build helpers that (de)serialize to native Go types, or add codegen to build immutable structs, sets, and lists from definitions -* I think we need a different mode for DataStore::Commit() to work in where it just fails if we were overly optimistic, instead of branching. For the user database, I never want to branch. I will just retry if I'm behind someone else. This will probably be useful for things like importers too, that don't care about concurrency. - -A little bad: -* The package name "datastore" is too long to be convenient. ds? -* We should make assert.Equals() call Value.Equals(). You can almost do this easily with Go interface magic, except that types also relies on dbg, so it's a circular dependency. -* Seems like ChunkStore::Writer::Close() should also commit the write, like how Go's File does. Right now, only Ref() does, so if you don't need the ref, you won't actually commit -* MemoryStore should clear the buf rather than invalidating on close, to be like FileStore and S3Store -* S3Store should really be called AWSStore since it depends on Dynamo too diff --git a/chunks/memory_store.go b/chunks/memory_store.go index 03fb77fdc3..6d3d485df2 100644 --- a/chunks/memory_store.go +++ b/chunks/memory_store.go @@ -65,6 +65,7 @@ func (w *memoryChunkWriter) Ref() (ref.Ref, error) { func (w *memoryChunkWriter) Close() error { // Not really necessary, but this will at least free memory and cause subsequent operations to crash. + // BUG 17: Make this method consistent with other ChunkStore implementations. *w = memoryChunkWriter{} return nil } diff --git a/user/user.go b/user/user.go index 2639c0e4fd..807bb2eb2e 100644 --- a/user/user.go +++ b/user/user.go @@ -10,7 +10,7 @@ func GetUsers(ds datastore.DataStore) types.Set { if ds.Roots().Len() == 0 { return types.NewSet() } else { - // We don't ever want to branch the user database. Currently we can't avoid that, but we should change DataStore::Commit() to support that mode of operation. + // BUG 13: We don't ever want to branch the user database. Currently we can't avoid that, but we should change DataStore::Commit() to support that mode of operation. Chk.EqualValues(1, ds.Roots().Len()) return ds.Roots().Any().(types.Map).Get(types.NewString("value")).(types.Set)