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

(SIMP-4206) Added organization provider and updated datasource provider #86

Merged
merged 3 commits into from
Feb 2, 2018

Conversation

heliocentric
Copy link

  • Updated grafana datasource provider to allow datasources to be created
    on specific organizations
  • Added grafana organization provider to allow organizations to be
    created
  • Updated README to add more descriptive information for previous
    features as well as new

Fixes commercial/enterprise-meta#91
SIMP-4206 #close

README.md Outdated
##### `grafana_dashboard`
#### `grafana_organization`

In order to use the organization resource, add the followingto your manifest:
Copy link
Member

Choose a reason for hiding this comment

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

followingto -> following to

@@ -0,0 +1,119 @@
# Copyright 2015 Mirantis, Inc.
Copy link
Member

Choose a reason for hiding this comment

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

2015?

# Copyright 2015 Mirantis, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
Copy link
Member

Choose a reason for hiding this comment

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

is that actually needed? The whole project uses the Apache-2 license. We try to keep the header short and simple, and don't add the license text to all files.

@bastelfreak bastelfreak added the enhancement New feature or request label Jan 26, 2018
@bastelfreak
Copy link
Member

Hi @heliocentric, thanks for the PR!

  • I added a few inline comments, can you take a look at them?
  • rubocop isn't happy, can you fix the warnings? Maybe autofix works with bundle exec rake rubocop:auto_correct
  • Can you add a few tests for this?

- Updated grafana datasource provider to allow datasources to be created
on specific organizations
- Added grafana organization provider to allow organizations to be
created
- Updated README to add more descriptive information for previous
features as well as new

Fixes commercial/enterprise-meta#91
SIMP-4206 #close
@brandonrdn
Copy link

@bastelfreak With the new push from @heliocentric I believe I've fixed all of your concerns.

  • The Copyright notices have been removed from new files (I copied them over from the other files that were created by bfraser to keep consistency).
  • The syntax error has been fixed in the README
  • New spec test created for organization provider
  • Updated spec test for datasource provider
  • Fixed all issues that rubocop had (0 issues currently)

Please let me know if there is anything else needed to complete this merge.

@bastelfreak
Copy link
Member

Thanks, I will check the code later. I need to find out why travis isn't working for this PR.

@bastelfreak
Copy link
Member

github UI isn't showing me any tests, but travis tells me it tested:
https://travis-ci.org/voxpupuli/puppet-grafana/builds/333847788
Can you take a look at the rubocop issue?

@brandonrdn
Copy link

@bastelfreak which rubocop issue are you referring to? The travis tests you linked are from the original request 4 days ago, doesn't look like they've been re-run.

@bastelfreak bastelfreak closed this Feb 2, 2018
@bastelfreak bastelfreak reopened this Feb 2, 2018
@bastelfreak
Copy link
Member

now travis is running again, let's see if it works

@bastelfreak
Copy link
Member

@heliocentric @brandonrdn now we got issues :)

@brandonrdn
Copy link

Hey @bastelfreak, I fixed the failures with the spec tests, that was an issue on my end for testing before committing changes and thinking they passed. As of right now, all spec tests pass and rubocop has 0 issues. I'll be pushing up from my fork to the SIMP fork, so you should see the push soon.

@brandonrdn
Copy link

@bastelfreak The newest updates are pushed. You may need to restart Travis again.

@bastelfreak bastelfreak closed this Feb 2, 2018
@bastelfreak bastelfreak reopened this Feb 2, 2018
@brandonrdn
Copy link

So close. I have the fix for the last few rubocop fails. It isn't telling me locally that it's broken.

@bastelfreak
Copy link
Member

rubocop sitll isn't happy. Can you have a look? you can execute it locally with:

bundle exec rake rubocop

you can invoke the autofixmagic with:

bundle exec rake rubocop:auto_correct

you maybe need to install the gems, if you didn't do that:

bundle install --path .vendor/ --without development --without system_tests --without release

@brandonrdn
Copy link

@bastelfreak I'm not sure why it's doing this, but I did all 3 previously. I bundle installed, ran the rubocop check, auto_corrected what I could, and manually fixed the rest. When I pushed up my last changes rubocop showed 0 issues. Next push should work. Fingers crossed.

@bastelfreak bastelfreak closed this Feb 2, 2018
@bastelfreak bastelfreak reopened this Feb 2, 2018
@bastelfreak
Copy link
Member

now it looks good. Thanks for the work!

@bastelfreak bastelfreak merged commit c38b50f into voxpupuli:master Feb 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants