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

nsqd: make importable #325

Merged
merged 1 commit into from
Mar 15, 2014
Merged

nsqd: make importable #325

merged 1 commit into from
Mar 15, 2014

Conversation

kzvezdarov
Copy link
Contributor

The changes in this PR split the nsqd application into an executable and a package (called nsqd), in the same manner as nsqlookupd.

@mreiferson
Copy link
Member

@kzvezdarov thanks for getting this started.

It looks like travis is reporting test failures. Looking at the logs it seems like test.sh isn't able to start an nsqd instance... can you look into that?

@mreiferson
Copy link
Member

As to your question on the ML, once we've got everything working, etc. we'll squash down the commits.

@kzvezdarov
Copy link
Contributor Author

The last comit should fix the problem. test.sh was looking for the executable in the old location. I had an old executable lying around in nsqd which was causing the tests to pass and give me a false positive.

@mreiferson
Copy link
Member

@kzvezdarov thanks, travis is happy 😄

I've gone through the public API that this change will now expose (by running go doc . in the nsqd top-level directory) and I think it would be prudent to clean up a few things:

  • un-export ProtocolV2 struct
  • export func (n *NSQD) getStats() (so external packages can get stats directly)
  • un-export PeerInfo struct
  • un-export LookupPeer struct
  • un-export NewLookupPeer(...)
  • un-export IdentifyEvent struct
  • un-export IdentifyDataV2 struct
  • un-export GUID int64
  • un-export GUIDFactory struct
  • un-export DiskQueue struct
  • un-export NewDiskQueue(...)
  • un-export DummyBackendQueue struct
  • un-export NewDummyBackendQueue(...)
  • un-export Context struct
  • un-export ClientV2 struct
  • un-export NewClientV2(...)
  • un-export WriteMessageToBackend(...)

Let's see how the public API looks after that...

@kzvezdarov
Copy link
Contributor Author

On it.

@kzvezdarov
Copy link
Contributor Author

In unexporting the structs, should I also unexport their exported fields?

@mreiferson
Copy link
Member

Good question, I don't think that's necessary though.

@kzvezdarov
Copy link
Contributor Author

I can't reproduce the above error and I am not too sure what it means/where it could come from?

@mreiferson
Copy link
Member

Yea, don't worry about it, looks like a flakey test. I'll poke around separately.

@mreiferson
Copy link
Member

@kzvezdarov you wanna rebase/squash this down and see tests pass?

@kzvezdarov
Copy link
Contributor Author

Seems well this time.

@mreiferson
Copy link
Member

great, thanks for your contribution!

mreiferson added a commit that referenced this pull request Mar 15, 2014
@mreiferson mreiferson merged commit 76b9dcb into nsqio:master Mar 15, 2014
@mreiferson mreiferson mentioned this pull request Mar 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants