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

Fix grafana_dashboards #31

Merged
merged 3 commits into from
Mar 6, 2017
Merged

Conversation

c10l
Copy link

@c10l c10l commented Mar 2, 2017

This fixes #25, while also fixing syntax errors of some API requests and implements idempotency.

Cassiano Leal added 3 commits March 2, 2017 17:50
This commit fixes the crash on the to_s method.

It also fixes the API endpoint for dashboards with + character in the name
@bastelfreak
Copy link
Member

looks good to me. Could someone with a bit more ruby knowledge review this as well?

@Sukiyakijango
Copy link

My question would be.. add a pull request when all the features are added by other previews request.. what is wrong with #23 and #17?

But from first look.. you removed in the Type the to_s def.. witch will cause that you will have the whole json outputed on each change.. the fix would be to use the is_to_s def..

@c10l
Copy link
Author

c10l commented Mar 4, 2017

I don't mind any of this being done as other PRs, but those don't seem to be merged because of outstanding issue.

I just thought perhaps if a cleaner PR with no issues would at least get the module in a working state, then the other features could be added in by those who need them.

I'm not sure I understand the is_to_s thing but I'll have a crack at it to see what I find out.

In the present state, this module is not very useful but with the fixes for the to_s method and the idempotency, it works for most use cases.

@roidelapluie roidelapluie merged commit d94b4a8 into voxpupuli:master Mar 6, 2017
@roidelapluie
Copy link
Member

LGTM, further improvements are now possible in other pull requests.

Thanks.

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.

Error while creating dashboard
4 participants