-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add feedback to users of FlightIcon
when icon name is not correct
#580
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
ba564b6
to
1861954
Compare
🦋 Changeset detectedLatest commit: 55f38cb The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
38e35db
to
4ca0d21
Compare
otherwise the import of the index file would not work (see https://github.com/ef4/ember-auto-import#usage-from-addons)
4ca0d21
to
013d10e
Compare
FlightIcon
when icon name is not correctFlightIcon
when icon name is not correct
@@ -0,0 +1,5 @@ | |||
--- | |||
"@hashicorp/flight-icons": minor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: do you need two separate change logs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MelSumner using the yarn changelog
command, it was not clear (to me) how to apply different semver increments to the packages, so I kept it simple and run it twice (and this created two files). I don't think this should make a difference for changelog
(@Dhaulagiri can you confirm this?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be possible to combine them via yarn changelog
but in practice I don't think it'll make a difference: changesets will be able to figure it out similar to if these were part of completely separate changes.
📌 Summary
As follow-up of this Slack conversation I have created a small PR that adds a feedback for the consumers of FlightIcons if a non-existing icon name is provided.
Example of what a developer would see in the console:
![screenshot_1817](https://user-images.githubusercontent.com/686239/192805801-920cf37d-8e10-47d0-8a9c-e4b3bb3c49a1.png)
🛠️ Detailed description
In this PR I have:
build
step in the Flight icons pipeline , to add the generation of a "index" file with the list of all the Flight icon namesEmberFlightIcon
component/addon to throw an error if a non-existing icon name is usedember-auto-import
fromdevDependencies
todependencies
(otherwise the import of the index file would not work; see https://github.com/ef4/ember-auto-import#usage-from-addons)EmberFlightIcon
👀 How to review
👉 Review by files changed
Reviewer's checklist:
💬 Please consider using conventional comments when reviewing this PR.