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

Using JSON.stringify instead of toString for object types in parseValue #111

Merged
merged 3 commits into from
May 18, 2021

Conversation

Totalus
Copy link
Contributor

@Totalus Totalus commented May 12, 2021

Fixes #110

@marcusolsson
Copy link
Contributor

This is awesome! There seems to be a few linter issues. Could you take a look?

@Totalus
Copy link
Contributor Author

Totalus commented May 14, 2021

Yes, so how do I do that ? Because when I build the plugin locally (yarn build) it does not throw any errors, but the same command fails in the CI pipeline.

Do I just need to run prettier on the code ? I tried yesterday, but it did also modify lines and files that I did not touch, so I got confused. Or maybe the linting config is not the same on the CI and locally ? I'm not so familiar with linting.

@marcusolsson
Copy link
Contributor

Hm, that is weird. yarn build should indeed give you the linter errors locally. Prettier should do the job. When was the last time you ran yarn install? The Grafana team updates the prettier config in pretty much every release, so you want to make sure that you're running the same Grafana dependencies as the CI. Try running yarn install and then prettier.

If you don't want to spend the time troubleshooting this, I'd be happy to merge this and fix the linter issues on my end. I created a branch with the correct formatting here in case you want to compare: https://github.com/marcusolsson/grafana-json-datasource/compare/Totalus/main

@Totalus
Copy link
Contributor Author

Totalus commented May 17, 2021

When was the last time you ran yarn install?

5 days ago. I just removed node_modules, ran yarn build again and it still gives the same result.

$ yarn list | grep @grafana
├─ @grafana/[email protected]
├─ @grafana/[email protected]
│  ├─ @grafana/tsconfig@^1.0.0-rc1       
├─ @grafana/[email protected]
├─ @grafana/[email protected]
│  ├─ @grafana/[email protected]
│  ├─ @grafana/[email protected]
├─ @grafana/[email protected]   
├─ @grafana/[email protected]
│  ├─ @grafana/[email protected]
│  ├─ @grafana/[email protected]       
│  ├─ @grafana/tsconfig@^1.0.0-rc1       
│  ├─ @grafana/[email protected]
├─ @grafana/[email protected]
├─ @grafana/[email protected]
│  ├─ @grafana/[email protected]
│  ├─ @grafana/[email protected]       
│  ├─ @grafana/[email protected]
│  ├─ @grafana/tsconfig@^1.0.0-rc1

Seeing anything wrong ?

What command runs for checking linting if not prettier ? I could try running the linting manually, try to figure out why it does not work properly. Maybe the linting does not find the config file and falls back to a default config ?

@marcusolsson marcusolsson merged commit 37300f8 into grafana:main May 18, 2021
@marcusolsson
Copy link
Contributor

Merged and fixed the linter issues in main (159f54b). Many thanks for helping out here! ❤️

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.

Use JSON.strignify to convert objects to string instead of toString
2 participants