Factor out common Graphite code #1932
Conversation
Comments imply it should be this name in any event.
otoolep
force-pushed
the
ol_refactor
branch
2 times, most recently
from
5bcbd58 to
f41530d
Compare
|
After reviewing some recent changes by @corylanou it seem to me we've now got too much boilerplate when bringing up a Graphite server. This change factors it out into the Graphite package. @corylanou -- let me know what you think. |
| @@ -12,7 +12,7 @@ import ( | |||
|
|
|||
| // TCPServer processes Graphite data received over TCP connections. | |||
| type TCPServer struct { | |||
| server Server | |||
| server SeriesWriter | |||
There was a problem hiding this comment.
I'm not sure I like how these names now line up. I understand that we introduced another Server interface above. Might be better as GraphiteServer and then Server or something along that line. I can see this be confusing for the next developer.
There was a problem hiding this comment.
OK, I think I follow you. Let me take another pass at this. However, calling something GraphiteServer within the graphite package does not follow Go style.
By doing this the SeriesWriter interface stays focused and just has methods for writing data. Its name then remains coherent.
|
@corylanou -- take a look now. I think the key to this refactor is to keep I also added a |
| @@ -752,6 +752,14 @@ func (s *Server) CreateDatabase(name string) error { | |||
| return err | |||
| } | |||
|
|
|||
| // CreateDatabaseIfNotExists creates a new database if, and only if, it does not exist already. | |||
| func (s *Server) CreateDatabaseIfNotExists(name string) error { | |||
There was a problem hiding this comment.
nit: could use some tests around this new method.
|
One nit, update changelog, and 🚢 |
|
Merging now. Thanks @corylanou |
Factor out common Graphite code
No description provided.