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: include uppercase characters when normalising plugin id #142

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

jackw
Copy link
Collaborator

@jackw jackw commented Nov 14, 2022

What this PR does / why we need it:
Users can type all sorts into the prompts when scaffolding a new plugin. Right now the normalizeId function strips uppercase characters which creates incorrect IDs (see path of directory created below):

success Installed "@grafana/create-plugin@0.5.0" with binaries:
      - create-plugin
? What is going to be the name of your plugin? My Plugin
? What is the organization name of your plugin? HeyWesty
? How would you describe your plugin?
? What kind of plugin would you like?  panel
? Do you want to add Github CI and Release workflows? Yes
? Do you want to add a Github workflow for automatically checking "Grafana API compatibility" on PRs? Yes
✔  ++ /Users/jackwestbrook/Sandbox/plugin-tools-tests/eyesty-ylugin-panel/.config/.eslintrc

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

📦 Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install @grafana/create-plugin@0.5.1-canary.142.d32fe7f.0
# or 
yarn add @grafana/create-plugin@0.5.1-canary.142.d32fe7f.0

@jackw jackw added bug create-plugin related to the create-plugin tool patch Increment the patch version when merged release Create a release when this pr is merged labels Nov 14, 2022
@jackw jackw added this to the v1.0.0 milestone Nov 14, 2022
@jackw jackw self-assigned this Nov 14, 2022
@tolzhabayev
Copy link
Collaborator

tolzhabayev commented Nov 14, 2022

Should'nt we instead .toLowerCase() them?

@tolzhabayev
Copy link
Collaborator

I think we should in:

Copy link
Collaborator

@leventebalogh leventebalogh left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Why are we using the change-case, upper-case, etc packages instead of string.toLowerCase()? To cater for some unicode character mappings?

Nit: maybe we could just return with this?

return changeCase.lowerCase(`${newOrgName}-${newPluginName}-${type}`);

Copy link
Collaborator

@tolzhabayev tolzhabayev left a comment

Choose a reason for hiding this comment

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

see above

Copy link
Collaborator

@tolzhabayev tolzhabayev left a comment

Choose a reason for hiding this comment

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

forget about my messages above, we already do lowercase in return

@jackw jackw merged commit fbd247f into main Nov 14, 2022
@jackw jackw deleted the jackw/fix-normalize-id branch November 14, 2022 13:23
@grafanabot
Copy link
Contributor

🚀 PR was released in @grafana/create-plugin@0.5.1 🚀

@grafanabot grafanabot added the released This issue/pull request has been released. label Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
create-plugin related to the create-plugin tool patch Increment the patch version when merged release Create a release when this pr is merged released This issue/pull request has been released.
Projects
Development

Successfully merging this pull request may close these issues.

4 participants