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: Update customAllValue parameter type to string in dashboard variable docs #172

Closed
wants to merge 6 commits into from

Conversation

isarns
Copy link
Contributor

@isarns isarns commented Feb 28, 2024

Overview

This PR updates the documentation for dashboard variables to correct the customAllValue parameter's type. The parameter was previously documented as a boolean, but it is actually a string. This change aligns the documentation with the Grafana documentation and the expected input type.

Changes

  • The customAllValue parameter type has been corrected from bool to string across the documentation where it is referenced.

Impact

This change will help users by providing accurate information about the customAllValue parameter type, ensuring that dashboard variables are used effectively and as intended within Grafana dashboards.

Please review the changes and merge them if everything is in order.

@isarns isarns requested a review from a team February 28, 2024 10:42
@CLAassistant
Copy link

CLAassistant commented Feb 28, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ Duologic
✅ isarns
❌ chkp-isarn
You have signed the CLA already but the status is still pending? Let us recheck it.

@malcolmholmes
Copy link
Contributor

The documentation is generated from schemas in Grafana, so any error should be fixed there, or in Grafonnet veneers which overlay the generated code.

@isarns
Copy link
Contributor Author

isarns commented Feb 28, 2024

The documentation is generated from schemas in Grafana, so any error should be fixed there, or in Grafonnet veneers which overlay the generated code.

I noticed that the Grafana schemas do not include the customAllValue parameter, so I made changes in the custom directory. Unfortunately, I am unable to run the make generator due to the following error:

+ jrsonnet -J vendor -S -c -m /Users/PATH/TO/grafonnet/gen/grafonnet-v10.2.0/docs/ --exec '(import '\''doc-util/main.libsonnet'\'').render(import '\''/Users/PATH/TO/grafonnet/gen/grafonnet-v10.2.0/main.libsonnet'\'')'
TRACE: render.libsonnet:18 warning: annotation both defined as object and package
no such field: any
                                                    : field <any> access
    vendor/doc-util/./render.libsonnet:106:15-39    : function <hasFields> call
    vendor/doc-util/./render.libsonnet:106:19-106:20: if condition
                                                    : evaluating argument
    vendor/doc-util/./render.libsonnet:93:8-93:9    : function <std.join> call
    vendor/doc-util/./render.libsonnet:89:22-38     : function <toString> call
+ finish
+ rm -rf /var/folders/6d/ck0zlsjx3vsd4x9r42y6knvr0000gn/T/tmp.TvPDPcrlFX
make: *** [gen/grafonnet-latest] Error 1

@Duologic
Copy link
Member

I've made a PR back to align this PR with main: https://github.com/isarns/grafonnet/pull/1

@isarns
Copy link
Contributor Author

isarns commented Mar 21, 2024

@Duologic Thanks for that!
Got it merged.

How about the make generator issue I have encountered? I'm using an intel MAC, maybe it causes the issue?

@Duologic
Copy link
Member

no such field: any
I think you need to have a newer jrsonnet version.

@isarns
Copy link
Contributor Author

isarns commented Mar 21, 2024

no such field: any
I think you need to have a newer jrsonnet version.

I use this version
jrsonnet 0.4.2 which is the latest

@Duologic
Copy link
Member

Probably need master, that version is old, bleeding edge here 😇

@Duologic
Copy link
Member

The generate process should probably become containerized so we don't have these issues.

@isarns
Copy link
Contributor Author

isarns commented Mar 21, 2024

Probably need master, that version is old, bleeding edge here 😇

Yep that worked thanks! 😃

@Duologic
Copy link
Member

Mind signing the CLA?

@isarns
Copy link
Contributor Author

isarns commented Mar 28, 2024

@Duologic

I will open a new PR without the other account that i pushed with accidentally.

@isarns
Copy link
Contributor Author

isarns commented Mar 28, 2024

Opened #189

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.

5 participants