Move _post_read and _admin_post_read to specific impl directories #45
Conversation
| reply_destroy(&rep); | ||
| } | ||
|
|
||
| done: |
There was a problem hiding this comment.
no space before label? same below
There was a problem hiding this comment.
Fixed label indentation.
|
I don't know about you, but I feel I just read the same code four times. 😿 |
|
That's right :(. I think I just doubled the already duplicate code. Let me On Thursday, November 19, 2015, Sebastian Waisbrot notifications@github.com
|
|
What do you think about creating a directory common to twemcache and slimcache and moving worker_helper.c and admin_helper.c there, to reduce code-duplication? I believe we will run into more such instances of refactoring once we start adding redis. I will work on reducing the code duplication in worker.c and admin.c in another pull request. |
|
That's fine by me, but check with @thinkingfish |
|
Created issue #46 to investigate reducing some duplicate code b/w core/worker and core/admin modules. |
| @@ -2,6 +2,7 @@ add_subdirectory(core ${PROJECT_BINARY_DIR}/core) | |||
| add_subdirectory(protocol ${PROJECT_BINARY_DIR}/protocol) | |||
| add_subdirectory(storage ${PROJECT_BINARY_DIR}/storage) | |||
| add_subdirectory(time ${PROJECT_BINARY_DIR}/time) | |||
| add_subdirectory(twemcache_shared ${PROJECT_BINARY_DIR}/twemcache_shared) | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
+1 to your comment about including stream primitives. Let me know if you can find a simple and better way. I couldn't come up with one in the short time that I spent on it.
There was a problem hiding this comment.
@thinkingfish btw, the main reason for this pull request is mentioned in issue #44, if you haven't already seen it.
There was a problem hiding this comment.
do you mean "memcache shared" instead of twemcache?
if so, should this be under protocol/memcache?
This comment was marked as spam.
This comment was marked as spam.
| @@ -185,7 +117,7 @@ _admin_event_read(struct buf_sock *s) | |||
| _tcp_accept(s); | |||
| } else if (c->level == CHANNEL_BASE) { | |||
| _admin_read(s); | |||
| _admin_post_read(s); | |||
| admin_post_read(s); | |||
There was a problem hiding this comment.
does this compile? I don't see how admin.c would know about admin_post_read since you moved it to another .c file and haven't added any headers
There was a problem hiding this comment.
Yes, it does, with an implicit declaration generation.
I did think about adding a header file, but later thought it might be too much for just 1 function per file.
…basic slab metrics unit test: test_metrics_insert_basic
Cc array test 3
This is to address #44 , to support pingserver and redis.
But this also has the undesirable side-effect of duplicating the code in: