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

using PR elastic/gosigar#13 to add process user_id to topbeat #773

Closed
wants to merge 5 commits into from

Conversation

mike-the-automator
Copy link
Contributor

I'm using PR elastic/gosigar#13 to add process user_id to topbeat this addresses the following issues: elastic/topbeat/issues/36
#590

we're golang noobs so let us know if there are improvements to make before we're ready to merge.

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@@ -1,7 +1,7 @@
package main

import (
topbeat "github.com/elastic/beats/topbeat/beat"
topbeat "github.com/dr-toboggan/beats/topbeat/beat"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not a good idea as it would mean our topbeat is built based on your repo :-) Please change it back to elastic.

@ruflin
Copy link
Contributor

ruflin commented Jan 19, 2016

@Dr-Toboggan Ignore my previous comments, I missed your comment on the PR. Lets merge your PR to gosigar first. Afterwards lets update the imports of this PR to point to the elastic repo.

@ruflin ruflin added the review label Jan 19, 2016
@andrewkroh
Copy link
Member

The PR to elastic/gosigar has been merged. Also I implemented Username for linux and OSX. So now you can proceed with updating the vendored copy of gosigar (and switch the imports back to elastic). Thanks!

@tsg
Copy link
Contributor

tsg commented Jan 25, 2016

@Dr-Toboggan Could you rebase this to master and update the gosigar version? Let me know if need help with it, I'd be happy to take over.

@mike-the-automator
Copy link
Contributor Author

I believe I've made the necessary changes and committed them. I struggled a bit with git and how to properly clone/modify the code. Please let me know if I need to change anything. Also, I've signed the CLA, but I'm still seeing the warning re: needing to sign it.

@tsg
Copy link
Contributor

tsg commented Jan 25, 2016

I can confirm the CLA signing, thanks! I'll try merging this one at the CLI via a separate PR.

@tsg
Copy link
Contributor

tsg commented Jan 25, 2016

Merging via #845. Thanks @Dr-Toboggan for contributing!

@tsg tsg closed this Jan 25, 2016
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.

5 participants