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

new icon: numpy (original, original-wordmark) #783

Merged
merged 9 commits into from
Aug 9, 2021

Conversation

kimjiwook0129
Copy link
Contributor

new icon: numpy (original, original-wordmark)

I've re-made the PR and made it from the develop branch.

I know this must be such simple task to do, but took me some time to learn some git tools and understand them since it is my first time trying to contribute to an open-source. But thanks for waiting me and for letting me to contribute!

Please feel free to tell me if anything still needs to be worked on.

Thanks for helping me out!

@roythearsonist
Copy link
Contributor

Looks good, but I'm a little confused. On the xml declaration the version is 1.0. But on the svg tag it says the version is 1.1. Any reason for that?

@kimjiwook0129
Copy link
Contributor Author

Hmm... Not too sure about the reason behind,
I think one of the versions should have been created while croping and the other during strokes-to-path removal (I did these 2 steps separately).
Shall I make them consistent?

- <?xml version="1.0" encoding="UTF-8"?><svg width="128pt" height="128pt" version="1.1" ...
+ <?xml version="1.1" encoding="UTF-8"?><svg width="128pt" height="128pt" version="1.1" ...

OR I can just delete the attribute if they are unnecessary:

- <?xml version="1.0" encoding="UTF-8"?><svg width="128pt" height="128pt" version="1.1" ...
+ <?xml encoding="UTF-8"?><svg width="128pt" height="128pt" ...

@roythearsonist
Copy link
Contributor

Uh I'm not a team member, I'm just some guy with experience with failing miserably at svgs. But to answer your question. You can remove the declaration (most svgs don't have it) and keep the version attribute

@kimjiwook0129
Copy link
Contributor Author

Declarations removed like other svgs, thanks!

@Panquesito7 Panquesito7 added the feature:icon Use this label for pull requests when a new icon is ready to be added to the collection label Aug 6, 2021
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Good work! Almost there. 😄

devicon.json Show resolved Hide resolved
devicon.json Outdated
]
},
"color": "#4DABCF",
"aliases": []
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"aliases": []
"aliases": [
{
"base": "original",
"alias": "plain"
}
]

@kimjiwook0129
Copy link
Contributor Author

Edited!

@Panquesito7 Panquesito7 added the bot:peek Use this label to trigger peek-bot. Remove and re-add the label to re-trigger label Aug 6, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2021

Hi there,

I'm Devicons' Peek Bot and I just peeked at the icons that you wanted to add using icomoon.io.
Here is the result below (top left):

Imgur Images

Here are the zoomed-in screenshots of the added icons:
Imgur ImagesImgur Images

Note: If the images don't show up, it's probably because it has been autodeleted by Imgur after 6 months due to our API choice.

The maintainers will now take a look at it and decide whether to merge your PR.

Thank you for contributing to Devicon! I hope everything works out and your icons are accepted into the repo.

Cheers,
Peek Bot 😊

Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

LGTM. Amazing work, @kimjiwook0129; thank you for your contribution! 😄👍🎉

@Thomas-Boi
Copy link
Member

@kimjiwook0129 the icons look great and ready to be merge. However, I'm almost done with a peek-bot upgrade that I want to test on this PR. Pretty much, Icomoon has issues with strokes in SVGs. Yours don't have them BUT they do use stroke=none. I just want to test whether Icomoon would have an issue with this.

Give me a week to get this done. After that, I'll merge this PR in whether the testing works since in theory, your SVGs are good to go.

@kimjiwook0129
Copy link
Contributor Author

Panquesito7

Thank you!

Thomas-Boi

Please take your time, and thank you!

Copy link
Member

@Thomas-Boi Thomas-Boi left a comment

Choose a reason for hiding this comment

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

Hi @kimjiwook0129,

I'm going to merge your PR into develop. The feature I'm working requires the next release but I want your icons in that release as well. Thus, I've tested your icon manually to support my feature and all is well 😄. Thank you for your icons and we appreciate your work 👍

@Thomas-Boi Thomas-Boi merged commit f8c5ddc into devicons:develop Aug 9, 2021
@amacado amacado mentioned this pull request Aug 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:peek Use this label to trigger peek-bot. Remove and re-add the label to re-trigger feature:icon Use this label for pull requests when a new icon is ready to be added to the collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants