Skip to content
This repository was archived by the owner on Nov 8, 2022. It is now read-only.

Fixes #67 - use snap-plugin-lib-go #68

Merged
merged 1 commit into from
May 23, 2017
Merged

Fixes #67 - use snap-plugin-lib-go #68

merged 1 commit into from
May 23, 2017

Conversation

jcooklin
Copy link
Collaborator

@jcooklin jcooklin commented May 18, 2017 •
edited
Loading

Fixes: #67
Relates: #69

  • Refactor to use plugin lib go
  • Updates deps
  • Adds debug logging for timing

Testing done:

  • manual
  • unit

img

@jcooklin jcooklin mentioned this pull request May 22, 2017
Open
candysmurf
candysmurf reviewed May 23, 2017
View reviewed changes
Copy link
Contributor

@candysmurf candysmurf left a comment

Choose a reason for hiding this comment

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

changes look good but both master and your PR don't load on my MAC. I can show you tomorrow.

main.go Outdated
psutil.NewPsutilCollector(), // CollectorPlugin interface
os.Args[1],
)
log.SetLevel(log.DebugLevel)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intended log level: debug?

rashmigottipati
rashmigottipati approved these changes May 23, 2017
View reviewed changes
Copy link

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

This PR looks good to me. Unable to reproduce the timeout error. Load works just fine.
LGTM after changing plugin version to 11.

@jcooklin jcooklin force-pushed the fb/updates-plugin-lib branch from 661b9da to 9758d92 Compare May 23, 2017 21:14
@jcooklin
Fixes intelsdi-x#67 - use snap-plugin-lib-go
a8aa848 
- Refactor to use plugin lib go
- Updates deps
- Adds debug logging for timing
@jcooklin jcooklin force-pushed the fb/updates-plugin-lib branch from 9758d92 to a8aa848 Compare May 23, 2017 21:46
@jcooklin jcooklin merged commit f4274aa into intelsdi-x:master May 23, 2017
@katarzyna-z katarzyna-z mentioned this pull request May 30, 2017
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@candysmurf candysmurf candysmurf left review comments

@rashmigottipati rashmigottipati rashmigottipati approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jcooklin @rashmigottipati @candysmurf