-
Notifications
You must be signed in to change notification settings - Fork 0
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
feature(icon-export): add batching and multi page processing #23
feature(icon-export): add batching and multi page processing #23
Conversation
✅ Deploy Preview for pine-icons ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
LGTM! 👍🏼
scripts/helpers.ts
Outdated
@@ -89,7 +89,6 @@ export const reservedKeywords = new Set([ | |||
'throw', | |||
'while', | |||
'yield', | |||
'delete', |
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.
Was removing this intentional?
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.
ugh...No, but Design does have an icon named delete
. We will need them to address the conflict. Great catch 🦅 👁️ 😂
request in batches
ae1fe2a
to
9023f1f
Compare
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.
🙌🏻
Description
As design looks to increase the number of icons in the library, the tool needed to be refactored to handle accordingly.
Solution