Skip to content

Conversation

@daczarne
Copy link

@daczarne daczarne commented Apr 16, 2022

Double check these details before you open a PR

  • PR does not match another non-stale PR currently opened
  • PR name matches the format new icon: Icon name (versions separated by comma). More details here
  • PR's base is the develop branch.
  • Your icons are inside a folder as seen here
  • SVG matches the standards laid out here
  • A new object is added in the devicon.json file as seen here

This PR closes NONE

Link to prove your SVG is correct and up-to-date.

The SVG itself I downloaded it from the YAML Wikipedia page and used the tools listed in this repo to fix it to Devicon standards. But, as shown in The YAML project GitHub organization, it is the correct and official one.

@daczarne daczarne changed the base branch from master to develop April 16, 2022 13:53
@daczarne daczarne changed the title New icon: YAML New icon: YAML (original) Apr 16, 2022
@daczarne daczarne marked this pull request as ready for review April 16, 2022 14:10
@daczarne daczarne changed the title New icon: YAML (original) New icon: YAML (original, plain-black, plain-white) Apr 16, 2022
@amacado amacado added the feature:icon PR when a new icon is ready to be added to the collection label Apr 17, 2022
Copy link
Member

@amacado amacado left a comment

Choose a reason for hiding this comment

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

hi! :) Thank you for picking up yaml. While looking through the files I noticed some things you might want to apply/fix before this pr is ready:

  • You introduced many new whitespaces in the devicon.json, while this is probably auto-added by a style in your idea: Please remove the whitespaces to follow our previous style
  • As our bot noticed there's also no font definition in the devicon.json for yaml and "SVG" must be be written lowercase :)
  • The icon itself looks non-centered. It seems like there is some space bellow but non above.
  • Also it looks like there is a small fine square around the logo (which is non intended). This might be a error in the github preview as well

You might take an additional look at https://github.com/devicons/devicon/wiki/Overview-on-Submitting-Icons and/or https://github.com/devicons/devicon/wiki/Example-of-Submitting-Icons-to-the-Repository

image

greetings and happy easter! 🐰

@daczarne
Copy link
Author

daczarne commented Apr 17, 2022

  • You introduced many new whitespaces in the devicon.json, while this is probably auto-added by a style in your idea: Please remove the whitespaces to follow our previous style

Yes, it looks like the VSCode JSON formatter linted the file. Can you tell me which lintter are you guys using?

  • As our bot noticed there's also no font definition in the devicon.json for yaml and "SVG" must be written in lowercase :)

I think I now understand what the font key is there for. I'll add it. The capitalization of the "SVG" key is in the Wiki. I would advise updating it if it's not necessary.

  • The icon itself looks non-centered. It seems like there is some space below but non-above.

I understand. But to my (extremely limited) knowledge of SVG that is a consequence of using viewBox="0 0 128 128". I should change the x and y coordinates in order to center it. Could you point to another way of doing it, @amacado?

  • Also it looks like there is a small fine square around the logo (which is non-intended). This might be an error in the GitHub preview as well

I don't see that square thb. Could you point to it in the SVG code, please?

@github-actions
Copy link
Contributor

Hi!

I'm the check-bot and we have some issues with your PR:

Error found in 'devicon.json' for 'yaml' entry: 
- Invalid version name in versions['svg']: 'plain-black'. Must match regexp: (original|plain|line)(-wordmark)?
- Invalid version name in versions['svg']: 'plain-white'. Must match regexp: (original|plain|line)(-wordmark)?
- Invalid version name in versions['font']: 'plain-black'. Must match regexp: (original|plain|line)(-wordmark)?
- Invalid version name in versions['font']: 'plain-white'. Must match regexp: (original|plain|line)(-wordmark)?

SVG Error in 'yaml-plain-black.svg':
- SVG file name didn't match our pattern of `name-(original|plain|line)(-wordmark)?.svg`

SVG Error in 'yaml-plain-white.svg':
- SVG file name didn't match our pattern of `name-(original|plain|line)(-wordmark)?.svg`

Check our CONTRIBUTING guide for more details regarding these errors.

Please address these issues. When you update this PR, I will check your SVGs again.

Thanks for your help,
SVG-Checker Bot 😄

Removes `plain-white` and `plain-black` versions
@Panquesito7 Panquesito7 added the bot:peek Trigger peek-bot. Remove and re-add the label to re-trigger label Apr 19, 2022
@github-actions
Copy link
Contributor

Hi there,

I'm Devicons' Peek Bot and I just peeked at the icons that you wanted to add using icomoon.io.

Here are the SVGs as intepreted by Icomoon when we upload the files:
Imgur Images

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

Here are the icons that will be generated by Icomoon:
Imgur Images

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

Here are the colored versions:
Imgur Images

The maintainers will now check for:

  1. The number of Glyphs matches the number of SVGs that were selected.
  2. The icons (second group of pictures) look the same as the SVGs (first group of pictures).
  3. The icons are of high quality (legible, matches the official logo, etc.)

In case of font issues, it might be caused by Icomoon not accepting strokes in the SVGs. Check this doc for more details and fix the issues as instructed by Icomoon and update this PR once you are done.

Thank you for contributing to Devicon! I hope that your icons are accepted into the repository.

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

Cheers,
Peek Bot 😊

@Panquesito7 Panquesito7 changed the title New icon: YAML (original, plain-black, plain-white) new icon: YAML (original) Apr 19, 2022
@Panquesito7 Panquesito7 requested a review from amacado April 19, 2022 14:51
Copy link
Member

@amacado amacado left a comment

Choose a reason for hiding this comment

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

I'm not an expert in svg either. All I can see is that the icon is not centered and should either cover the whole space or be centered. Currently there's space at the bottom. You can see that clearly in the icon preview:

image

@daczarne daczarne closed this Apr 19, 2022
@daczarne daczarne deleted the new-icon-yaml branch April 19, 2022 22:16
@KayleeDavisGitHub KayleeDavisGitHub mentioned this pull request Oct 11, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:peek Trigger peek-bot. Remove and re-add the label to re-trigger feature:icon PR 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.

3 participants