-
Notifications
You must be signed in to change notification settings - Fork 231
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
replace master/slave naming convention with coordinator/worker #650
base: master
Are you sure you want to change the base?
Conversation
README.md
Outdated
@@ -1,7 +1,7 @@ | |||
# Zeus | |||
|
|||
[![Join the chat at https://gitter.im/zeus-application-preloader/Lobby](https://badges.gitter.im/zeus-application-preloader/Lobby.svg)](https://gitter.im/zeus-application-preloader/Lobby?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) | |||
[![Build Status](https://travis-ci.org/burke/zeus.svg?branch=master)](https://travis-ci.org/burke/zeus) | |||
[![Build Status](https://travis-ci.org/burke/zeus.svg?branch=coordinator)](https://travis-ci.org/burke/zeus) |
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.
💭 is this really what you want?
|
||
fd = ENV['ZEUS_MASTER_FD'].to_i | ||
self.master_socket = UNIXSocket.for_fd(fd) |
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.
typo on this line (closing paren shouldn't be in the right square bracket)
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.
Meh, I had fixed this locally to get tests passing but failed to commit. My bad!
now: I don't know why test are failing, but FWIW I'm a fan of this change, however difficult it's going to make merging with any open branches. |
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.
I think the tests are failing due to MacOS oddities I have seen this before and they might be flaky on OSX so I restarted them just to see (probably not something to fix in this PR). In general I'm in favor of this but as it is a massive change and hard to review and I'm currently not using zeus anymore I don't feel comfortable merging it.
@@ -25,7 +25,7 @@ module Zeus | |||
# | |||
#[![m ci](https://secure.travis-ci.org/qrush/m.png)](http://travis-ci.org/qrush/m) | |||
# | |||
#![Rush is a heavy metal band. Look it up on Wikipedia.](https://raw.github.com/qrush/m/master/rush.jpg) | |||
#![Rush is a heavy metal band. Look it up on Wikipedia.](https://raw.github.com/qrush/m/coordinator/rush.jpg) |
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.
I think the find and replace went to far here as this is not part of this repo.
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.
You are correct, I'll adjust.
Description of Problem
Typical story: team loves software, situation comes up where team learns their favorite software happens to use antiquated terminology, ... so an attempt to improve that. Hoping it'll be more like the Mozilla/Django story than Redis.
Observed Behavior
Expected Behavior