Added check script for the icons and fix icons with fill or viewBox issues

[Thomas-Boi]

Hello,

The peek script now checks for the svg's viewBox, height, width, and whether it's using <style>. The CONTRIBUTING.md is also updated with the new standards.

This PR also contain the fix for the icons as seen in this comment

[Thomas-Boi]

Do you mean that it would trigger on pull request, not just when the tag is added?

I think we can actually change that now for the bot:peek script as well. Since all feature:icon PR has the same title, we can just run the script on pull request and if it has the title in our format.

Now, I don't mind separating the two scripts. However, there are many similar steps between them, such as finding which icons were added, so I'd like to keep them the same as much as possible.

[Thomas-Boi]

Oh, two more things:

-I'll also add some checks for the enable-background, the x and y, and the comments -I'm thinking whether we should create some kind of scripts that can run locally to fix the above issues. Since removing them won't create issues, we can do that programmatically to offset the burdens on the contributors. Just a thought.

[amacado]

Do you mean that it would trigger on pull request, not just when the tag is added?

I think we can actually change that now for the bot:peek script as well. Since all feature:icon PR has the same title, we can just run the script on pull request and if it has the title in our format.

Now, I don't mind separating the two scripts. However, there are many similar steps between them, such as finding which icons were added, so I'd like to keep them the same as much as possible.

Separated checks would have the advantage of setting one as "required" and another to optional. For example a pull request which does not contain a new icon would set the check to a "failed" state which is not intended when there is no new icon. So we would need to setup a "skip" the check action when no icon is added (which ends up recursive because checking the title of the pull request would be a in the bot:peek). So I think it's better to separate the tasks in different workflows.

-I'm thinking whether we should create some kind of scripts that can run locally to fix the above issues. Since removing them won't create issues, we can do that programmatically to offset the burdens on the contributors. Just a thought.

Sure, any automated help we can provide is fine! 👍🏻

[Thomas-Boi]

Separated checks would have the advantage of setting one as "required" and another to optional. For example a pull request which does not contain a new icon would set the check to a "failed" state which is not intended when there is no new icon. So we would need to setup a "skip" the check action when no icon is added (which ends up recursive because checking the title of the pull request would be a in the bot:peek). So I think it's better to separate the tasks in different workflows.

Ok, I see the benefit of splitting the workflows. However, shouldn't we have some kind of filter for this check instead of just triggering on every pull_request? Some PR might not have any svgs involve, such as script related stuff. This would still trigger the script. However, there is no penalty for running actions multiple time so it's up to you.

As for the checking itself: I can set up the script so it checks the old way. Compare the devicon.json and icomoon.json, any difference will be checked etc... so it's not too bad.

[Thomas-Boi]

I just added the check_svgs workflow to the branch. It will be triggered whenever a PR is opened. Unfortunately, I couldn't find a way to fix the icons' extra info yet. There's some possibility in terms of gulp tasks but I'm trying to figure out when to do it. We have two options:

-Put it in the build task. -Trigger a script that do this after the PR is merged.

What do you think?

[amacado]

Some PR might not have any svgs involve, such as script related stuff. This would still trigger the script. However, there is no penalty for running actions multiple time so it's up to you.

That's true, but since the scripts purpose is to check not only new icons but all it's fine I think. The execution should not take too long, or?

I couldn't find a way to fix the icons' extra info yet. There's some possibility in terms of gulp tasks but I'm trying to figure out when to do it.

I'm not sure if I understand what you mean. Can you explain a little more about "icons extra info"?

[Thomas-Boi]

That's true, but since the scripts purpose is to check not only new icons but all it's fine I think. The execution should not take too long, or?

Oh, are we checking all icons in the repo whenever we pull? I have it set up rn so it only pulls the one that hasn't been made into an icon yet. I think this is better than checking the entire /icons folder so we get the results back faster.

I'm not sure if I understand what you mean. Can you explain a little more about "icons extra info"?

I was referring to the enable-background, x, y, and comments in the svg files.

[Thomas-Boi]

Hey @amacado ,

Can I merge this PR in? I don't think there's much of a time/performance difference between checking all svgs and checking all non-iconized svgs. I also don't plan on adding the ability to remove comments in this PR.

[github-actions[bot]]

Hi!

I'm Devicons' Peek Bot and it seems we've ran into a problem. I'm supposed to check your svgs but I couldn't do my task due to an issue.

Please double check and fix the possible issues below:

• Your svgs are named and added correctly to the /icons folder as seen here.
• Your icon information has been added to the devicon.json as seen here
• Your PR title follows the format seen here

Once everything is fixed, the maintainers will try again. If I still fail, the maintainers will investigate what cause this problem.

Thank you for your help :smile:

Cheers :),

Peek Bot

[Thomas-Boi]

Don't merge yet. Something weird is happening

[github-actions[bot]]

Hi!

I'm Devicons' Peek Bot and it seems we've ran into a problem. I'm supposed to check your svgs but I couldn't do my task due to an issue.

Please double check and fix the possible issues below:

• Your svgs are named and added correctly to the /icons folder as seen here.
• Your icon information has been added to the devicon.json as seen here
• Your PR title follows the format seen here

Once everything is fixed, the maintainers will try again. If I still fail, the maintainers will investigate what cause this problem.

Thank you for your help :smile:

Cheers :),

Peek Bot

[Thomas-Boi]

I figured out why. Turns out one of the icons (sqlalchemy) has invalid svgs. I think I can fix this in this commit too. Until then, we should refrain from merging until this gets done.

I'll also update the comment bot. Right now the message is not very helpful to us

[github-actions[bot]]

Hi!

I'm Devicons' SVG-Checker Bot and it seems we've ran into a problem. I'm supposed to check your svgs but I couldn't do my task due to an issue.

Here is what went wrong:

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

Thanks for your help,

SVG-Checker Bot :smile:

[github-actions[bot]]

Hi!

I'm Devicons' SVG-Checker Bot and it seems we've ran into a problem. I'm supposed to check your svgs but I couldn't do my task due to an issue.

Here is what went wrong:

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

Thanks for your help,

SVG-Checker Bot :smile:

[github-actions[bot]]

Hi!

I'm Devicons' SVG-Checker Bot and it seems we've ran into a problem. I'm supposed to check your svgs but I couldn't do my task due to an issue.

Here is what went wrong:

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

Thanks for your help,

SVG-Checker Bot :smile:

[github-actions[bot]]

Hi!

I'm Devicons' SVG-Checker Bot and it seems we've ran into a problem. I'm supposed to check your svgs but I couldn't do my task due to an issue.

Here is what went wrong:

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

Thanks for your help,

SVG-Checker Bot :smile:

[github-actions[bot]]

Hi!

I'm Devicons' SVG-Checker Bot and it seems we've ran into a problem. I'm supposed to check your svgs but I couldn't do my task due to an issue.

Here is what went wrong:

[github-actions[bot]]

Hi!

I'm Devicons' SVG-Checker Bot and it seems we've ran into a problem. I'm supposed to check your svgs but I couldn't do my task due to an issue.

Here is what went wrong:

sqlalchemy-original.svg:
-uses 'enable-background' in its style. This is deprecated
-has an 'x' attribute, this is not needed
-has an 'y' attribute, this is not needed

sqlalchemy-original-wordmark.svg:
-uses 'enable-background' in its style. This is deprecated
-has an 'x' attribute, this is not needed
-has an 'y' attribute, this is not needed

sqlalchemy-plain.svg:
-'viewBox' is not '0 0 128 128'
-uses 'enable-background' in its style. This is deprecated
-has an 'x' attribute, this is not needed
-has an 'y' attribute, this is not needed

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

Thanks for your help,

SVG-Checker Bot :smile:

[github-actions[bot]]

Hi! I'm Devicons' SVG-Checker Bot. All of your svgs seem fine.

Thanks for your help,

SVG-Checker Bot :smile:

[amacado]

Hi!

I'm Devicons' SVG-Checker Bot and it seems we've ran into a problem. I'm supposed to check your svgs but I couldn't do my task due to an issue.

Here is what went wrong:

sqlalchemy-original.svg:
-uses 'enable-background' in its style. This is deprecated
-has an 'x' attribute, this is not needed
-has an 'y' attribute, this is not needed

sqlalchemy-original-wordmark.svg:
-uses 'enable-background' in its style. This is deprecated
-has an 'x' attribute, this is not needed
-has an 'y' attribute, this is not needed

sqlalchemy-plain.svg:
-'viewBox' is not '0 0 128 128'
-uses 'enable-background' in its style. This is deprecated
-has an 'x' attribute, this is not needed
-has an 'y' attribute, this is not needed

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

Thanks for your help,

SVG-Checker Bot 😄

OMG! This is so cool! Love it! :heart:

[Thomas-Boi]

Thanks! I figured that this would save time from copy the run log over to a new comment.

I think I'm done with this PR for now. I was looking for some way to upload images but no such luck.

[github-actions[bot]]

Hi! I'm Devicons' SVG-Checker Bot and I just checked all the SVGs in this branch.

Everything looks great. Good job!

Have a nice day, SVG-Checker Bot :grin:

[github-actions[bot]]

Hi! I'm Devicons' SVG-Checker Bot and I just checked all the SVGs in this branch.

Everything looks great. Good job!

Have a nice day, SVG-Checker Bot :grin: