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.

Sign up for GitHub

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

Jump to bottom

Multi-datasource dashboards & instructions to load dashboards in Grafana config #323

Merged
merged 8 commits into from
May 1, 2023
Merged

Multi-datasource dashboards & instructions to load dashboards in Grafana config #323

merged 8 commits into from
May 1, 2023

Conversation

jkaflik
Copy link
Collaborator

@jkaflik jkaflik commented Mar 14, 2023

No description provided.

@jkaflik jkaflik changed the title Multi-datasource dashboards & README for Grafana config Multi-datasource dashboards & instructions to load dashboards in Grafana config Mar 15, 2023
@jkaflik jkaflik marked this pull request as ready for review March 15, 2023 06:21
@jkaflik jkaflik requested a review from a team as a code owner March 15, 2023 06:21
bossinc
bossinc reviewed Mar 20, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
src/dashboards/cluster-analysis.json Show resolved Hide resolved
@bossinc
Copy link
Collaborator

bossinc commented Mar 31, 2023

@jkaflik I do not think adding links to the premade dashboards in the docs is helpful. The docs already described how to add the premade dashboards. Please remove the README.md edits and I'll approve this PR. 🙂

@kgoralski
Copy link
Contributor

kgoralski commented Mar 31, 2023 •
edited

@jkaflik I do not think adding links to the premade dashboards in the docs is helpful. The docs already described how to add the premade dashboards. Please remove the README.md edits and I'll approve this PR. 🙂

imo reams edits are helpful, it's a quick info how to include the dashboards for this plugin. Without the readme edits is not clear how to do that and you need few additional link clicks to learn about it. And the import dashboards docs are general docs and not specific ones from here.

With readme edits it's self contained and less confusing for the reader.

@jkaflik
Copy link
Collaborator Author

jkaflik commented Apr 3, 2023

README revert

@jkaflik
Copy link
Collaborator Author

jkaflik commented Apr 12, 2023

@bossinc @asimpson can you folks have a look? thanks

asimpson
asimpson approved these changes Apr 14, 2023
View reviewed changes
Copy link
Contributor

@asimpson asimpson left a comment

Choose a reason for hiding this comment

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

Nice work on this! 🚀

@asimpson
Copy link
Contributor

@jkaflik I'm happy to merge this but I wanted to check with you, anything else you wanted to add?

@jkaflik
Copy link
Collaborator Author

jkaflik commented Apr 17, 2023

@asimpson all good. let's merge

@bossinc bossinc merged commit c5e9eae into grafana:main May 1, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bossinc bossinc bossinc left review comments

@kgoralski kgoralski kgoralski left review comments

@gingerwizard gingerwizard gingerwizard left review comments

@asimpson asimpson asimpson approved these changes

@alyssabull alyssabull Awaiting requested review from alyssabull alyssabull is a code owner automatically assigned from grafana/partner-datasources

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@jkaflik @bossinc @kgoralski @asimpson @gingerwizard