- Notifications
You must be signed in to change notification settings - Fork 9
Refactoring SwiftMemcache #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Package.swift Outdated
| dependencies: ["Memcache"] | ||
| ), | ||
| .executableTarget( | ||
| name: "swift-memcache-gsoc-example", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this to MemcacheExample
README.md Outdated
| | ||
| ## Getting Started | ||
| | ||
| ### Docker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this to a Contributing section. This is not relevant for users of the package.
| @@ -12,7 +12,11 @@ | |||
| // | |||
| //===----------------------------------------------------------------------===// | |||
| | |||
| import Foundation | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename the file as well
3595df2 to f3aba8e Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last few nits!
| @@ -0,0 +1,72 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't commit this
| @@ -0,0 +1,140 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
| @@ -0,0 +1,104 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here :D
README.md Outdated
| | ||
| ### Memcached Connection API | ||
| | ||
| Our `MemcachedConnection` allows for communicate with a Memcached server. This actor takes care of establishing a connection, creating a request stream and handling asynchronous execution of commands. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is now MemcacheConnection right?
This PR addresses the need to align the library with Memcache naming conventions and to remove any unnecessary dependences. This PR closes #39.
Motivation:
To adhere to industry standard naming conventions by renaming types to be
memcachei.e without thememcache/d.Modifications:
Memcachedto now useMemcahce.Results:
With these changes the library is better aligned with Memcache naming conventions, making it more recognizable and idiomatic to the community.