grafana / plugin-tools

Create Grafana plugins with ease.
https://grafana.com/developers/plugin-tools/
Apache License 2.0
57 stars 28 forks source link

Create Plugins: Fix plugin id mismatch #834

Closed oshirohugo closed 5 months ago

oshirohugo commented 5 months ago

What this PR does / why we need it:

There was some mismatching in the pluginId created in the templates, this PR fix it.

Which issue(s) this PR fixes:

Fixes https://github.com/grafana/plugin-tools/issues/758

Special notes for your reviewer:

To test this, I create projects using the old version and the new version for all plugin types: app, datasource, panel and scenesapp (with all the options on). I compared the results, and they were the same for app, datasource and panel. For scenesapp the fixes made all the pluginIds to be the same and the export path new has the -app sufix instead of -scenesapp

📦 Published PR as canary version: Canary Versions
:sparkles: Test out this PR locally via: ```bash npm install @grafana/create-plugin@4.2.6-canary.834.966fae0.0 # or yarn add @grafana/create-plugin@4.2.6-canary.834.966fae0.0 ```
github-actions[bot] commented 5 months ago

Hello! 👋 This repository uses Auto for releasing packages using PR labels.

✨ This PR can be merged and will trigger a new patch release. NOTE: When merging a PR with the release label please avoid merging another PR. For further information see here.

oshirohugo commented 5 months ago

@leventebalogh regarding your suggestion:

One thing though: unfortunately we also need to update the getTemplateData() function in utils.templates.ts to make sure that the update and generate flows are the same.

Do you mean doing something like:

    pluginId: normalizeId(pluginJson.name, pluginJson.org, pluginJson.type),

?

If that's the case, couldn't it be an issue, since the plugin id will change? E.g. all the old scenesapp type plugins that have id with suffix -scenesapp will have its id changed to an id with suffix -app. If it's an update maybe the plugin is already in our catalog and the id is already in our database, so the update will introduce a breaking change.

oshirohugo commented 5 months ago

Closing PR in favor of https://github.com/grafana/plugin-tools/pull/837

@Ukochka merge this branch into hers.