Skip to content
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

Zk name service #42

Merged
merged 10 commits into from
Apr 17, 2014
Merged

Zk name service #42

merged 10 commits into from
Apr 17, 2014

Conversation

msolo
Copy link
Contributor

@msolo msolo commented Apr 16, 2014

No description provided.

@ryszard
Copy link
Contributor

ryszard commented Apr 16, 2014

The code itself looks good. However, we are trying to make the Vitess code compliant to the Go styleguide, which includes having doccomments of the form

// Foo foos the bar.
func Foo(bar Bar) {
...
}

In my experience this actually helps when you are trying to figure out what the code is doing. Can you please reword your doccomments in this spirit?

@msolo
Copy link
Contributor Author

msolo commented Apr 16, 2014

I'm surprised to hear you say that. Quickly scanning the diff, the vast majority of important functions and structures (particularly the public functions) are quite well documented.

Did you have a specific example where something in here needed more explanation? About the only example I can think of is the Election() method which is explained by comments on the structure itself.

alainjobart added a commit that referenced this pull request Apr 17, 2014
@alainjobart alainjobart merged commit 49fff35 into vitessio:master Apr 17, 2014
@msolo msolo deleted the zk-name-service branch April 17, 2014 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants