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

Thomas-Boi · 2021-01-04T23:17:50Z

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

amacado

I'm happy with those improvements! Thanks @Thomas-Boi.
Maybe we could run this new check on every pull request regardless of the bot:peek command? (see #407 (comment)). We would need to create a new workflow containing only this step but we could use this to restrict the merge if this check fails.

Thomas-Boi · 2021-01-05T23:35:40Z

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 · 2021-01-05T23:49:49Z

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 · 2021-01-06T01:14:02Z

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 · 2021-01-06T02:06:08Z

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 · 2021-01-06T05:45:56Z

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 · 2021-01-06T16:35:04Z

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 · 2021-01-06T18:26:48Z

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 · 2021-01-07T03:29:42Z

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 · 2021-01-07T21:04:39Z

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 😄

Cheers :),

Peek Bot

Thomas-Boi · 2021-01-07T21:16:03Z

Don't merge yet. Something weird is happening

github-actions · 2021-01-07T22:11:39Z

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 😄

Cheers :),

Peek Bot

Thomas-Boi · 2021-01-07T22:12:45Z

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 · 2021-01-07T23:34:19Z

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 😄

github-actions · 2021-01-07T23:46:58Z

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 😄

github-actions · 2021-01-07T23:51:05Z

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 😄

github-actions · 2021-01-07T23:54:53Z

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 😄

github-actions · 2021-01-08T00:01:32Z

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:

uses 'enable-background' in its style. This is deprecated: /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-original.svg
has an 'x' attribute, this is not needed: /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-original.svg
has an 'y' attribute, this is not needed: /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-original.svg
uses 'enable-background' in its style. This is deprecated: /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-original-wordmark.svg
has an 'x' attribute, this is not needed: /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-original-wordmark.svg
has an 'y' attribute, this is not needed: /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-original-wordmark.svg
'viewBox' is not '0 0 128 128': /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-plain.svg
uses 'enable-background' in its style. This is deprecated: /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-plain.svg
has an 'x' attribute, this is not needed: /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-plain.svg
has an 'y' attribute, this is not needed: /home/runner/work/devicon/devicon/icons/sqlalchemy/sqlalchemy-plain.svg

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

Thanks for your help,

SVG-Checker Bot 😄

github-actions · 2021-01-08T00:44:54Z

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 😄

github-actions · 2021-01-08T01:15:06Z

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

Thanks for your help,

SVG-Checker Bot 😄

amacado · 2021-01-08T01:20:30Z

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! ❤️

Thomas-Boi · 2021-01-08T01:43:49Z

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 · 2021-01-08T01:44:43Z

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 😁

github-actions · 2021-01-08T01:48:24Z

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 😁

* Create eleventy-original.svg * Create eleventy-plain.svg * Update devicon.json * Update eleventy-plain.svg * Update eleventy-original.svg * new icon: sqlalchemy (plain, original, original-workmark) * Add font aliase for sqlalchemy * new icon: microsoftsqlserver (plain, plain-wordmark) (#427) * new icon sqlserver (plain, plain-woodmark) * Update icons/sqlserver/sqlserver-plain-wordmark.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update icons/sqlserver/sqlserver-plain.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update devicon.json Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update devicon.json Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Rename sqlserver icons to microsoftsqlserver Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * new icon: kubernetes (plain, plain-wordmark) (#424) * new icon: kubernetes (plain, plain-wordmark) * Update icons/kubernetes/kubernetes-plain.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update icons/kubernetes/kubernetes-plain-wordmark.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * new icon: rocksdb (plain) (#423) * new icon: rocksdb (plain) * Update icons/rocksdb/rocksdb-plain.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * new icon: googlecloud (original, original-wordmark, plain, plain-wordmark) (#428) * new icon: googlecloud (plain, plain-wordmark, original, original-wordmark) * Update icons/googlecloud/googlecloud-plain.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update icons/googlecloud/googlecloud-original-wordmark.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update icons/googlecloud/googlecloud-original.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Fix google cloud icons * Fix googlecloud original * Remove fill redefinition from google cloud original * Fix googlecloud alignment (Test) * Fix googlecloud alignment (test) * Fix googlecloud alignment (test) Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * new icon: objectivec (plain) (#425) * new icon: objectivec (plain) * Update icons/objectivec/objectivec-plain.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * new icon uwsgi (original, plain) * Cleanup uwsgi icons * Update devicon.json Co-authored-by: Thomas Bui <43018778+Thomas-Boi@users.noreply.github.com> * Add adobe XD line icon * Add xd-plain and eps file * Clean up xd svg file * Update devicon.json * new icon: firebase (plain, plain-wordmark) (#461) * add icon Firebase (plain, plain-wordmark), Closes #204 * new icon: firebase (plain, plain-wordmark) * update icons * Add plain-wordmark to firebase fonts * new icon: flask (original, original-wordmark) (#463) * Cleanup flask icon * Improve flask icons * Update icons/flask/flask-original-wordmark.svg Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> Co-authored-by: moghya <sawantshubham571@gmail.com> Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update devicon.json * Fixed an error with devicon.json * Added check script for the icons and fix icons with fill or viewBox issues (#460) * Added script to check svg fill and viewBox * Fix cucumber-plain-wordmark * Fix various fill and viewBox issues in svgs * Added check for height and width attr * Added check_svgs workflow * Fix an issue where the error is not log properly * Added on push for testing * Updated trigger so it now runs whenever PR is update * Added sleep to script to make logs nicer * Added script that create env var * Updated the github_env to accomodate ubuntu * Change format of log and allow filehandler to return Path * Updated logging messages * Updated refs for the checkout action * Make logging nicer * Updated fix messaging so it's more clear * fix icons: icons/cucumber/cucumber-plain-wordmark.svg, icons/intellij/intellij-plain-wordmark.svg, icons/jenkins/jenkins-plain.svg, icons/twitter/twitter-original.svg, icons/yunohost/yunohost-plain.svg * Fix the ref issue of the checkout action and sqlalchemy (#472) * Remove head_ref from checkout action * Fixed the svg errs in sqlalchemy * Create a monthly script that checks all svgs * Python now print traceback * Updated file names in check_svgs_on_pr * Remove deprecated and unused svg syntax Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Remove deprecated and unused svg syntax Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update svg fill, remove unused class * Change comment action to a new action * Add guideline about squash merging As the result of discussion #470 update guidelines with squash merging * Redo the workflow archs so we can comment on pr * Moved check_svgs_monthly to draft * Added working workflow file * Changed to file read action * new icon: appwrite (plain, original, wordmark) (#371) * Added Appwrite icon * Fixed wrong JSON input * Fixed JSON style * Fixed indent * Update devicon.json Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update devicon.json Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * Update devicon.json Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> * bot:build new icons, icomoon.json and devicon.css (#486) * fix bash-plain (#451) (#453) Co-authored-by: Enis Mulić <enis.s.mulic@gmail.com> * Built new icons, icomoon.json and devicon.css Co-authored-by: Clemens Bastian <8781699+amacado@users.noreply.github.com> Co-authored-by: Enis Mulić <enis.s.mulic@gmail.com> Co-authored-by: amacado <amacado@users.noreply.github.com> * Removed 'default fall back icon' from build_icons.yml Co-authored-by: Tylen St Hilaire <28753109+tylensthilaire@users.noreply.github.com> Co-authored-by: Enis Mulić <enis.s.mulic@gmail.com> Co-authored-by: Thomas Bui <43018778+Thomas-Boi@users.noreply.github.com> Co-authored-by: Withee Poositasai <witheep@gmail.com> Co-authored-by: moghya <sawantshubham571@gmail.com> Co-authored-by: Thomas Bui <thomasbui198@gmail.com> Co-authored-by: Eldad A. Fux <eldad.fux@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: amacado <amacado@users.noreply.github.com>

Thomas-Boi added devops Use this label for devops related enhancements enhancement labels Jan 5, 2021

Thomas-Boi requested a review from amacado January 5, 2021 00:32

amacado approved these changes Jan 5, 2021

View reviewed changes

Thomas-Boi added 5 commits January 5, 2021 21:16

Added script to check svg fill and viewBox

b845998

Fix cucumber-plain-wordmark

208ca10

Fix various fill and viewBox issues in svgs

4a9d49c

Added check for height and width attr

1a0ec51

Added check_svgs workflow

5b43dad

Thomas-Boi force-pushed the TB_peekUpgrade branch from 0d0ae6e to 5b43dad Compare January 6, 2021 05:19

Thomas-Boi added 3 commits January 7, 2021 12:54

Fix an issue where the error is not log properly

0a49be1

Added on push for testing

b99d514

Updated trigger so it now runs whenever PR is update

9f01332

Added sleep to script to make logs nicer

c9cfca8

Added script that create env var

42047d9

Thomas-Boi force-pushed the TB_peekUpgrade branch from c5bda17 to ea359d3 Compare January 7, 2021 23:50

Thomas-Boi force-pushed the TB_peekUpgrade branch from ea359d3 to 643d84d Compare January 7, 2021 23:54

Updated the github_env to accomodate ubuntu

1c166eb

Thomas-Boi force-pushed the TB_peekUpgrade branch from 643d84d to 1c166eb Compare January 8, 2021 00:01

Change format of log and allow filehandler to return Path

4a072c8

Thomas-Boi added 2 commits January 7, 2021 16:56

Updated logging messages

e339961

Updated refs for the checkout action

d11ed9b

Make logging nicer

273a1a6

Updated fix messaging so it's more clear

4d53e88

Thomas-Boi requested a review from amacado January 8, 2021 06:06

amacado approved these changes Jan 8, 2021

View reviewed changes

amacado merged commit 82bccb1 into develop Jan 8, 2021

amacado deleted the TB_peekUpgrade branch January 8, 2021 20:36

amacado restored the TB_peekUpgrade branch January 8, 2021 20:51

amacado mentioned this pull request Jan 8, 2021

workflows on pull requests failing due to wrong ref #471

Closed

Thomas-Boi mentioned this pull request Jan 8, 2021

Fix the ref issue of the checkout action and sqlalchemy #472

Merged

Thomas-Boi deleted the TB_peekUpgrade branch January 8, 2021 23:01

amacado mentioned this pull request Jan 19, 2021

Release 2.9.0 #485

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Thomas-Boi commented Jan 4, 2021

amacado left a comment

Thomas-Boi commented Jan 5, 2021

Thomas-Boi commented Jan 5, 2021

amacado commented Jan 6, 2021

Thomas-Boi commented Jan 6, 2021

Thomas-Boi commented Jan 6, 2021

amacado commented Jan 6, 2021

Thomas-Boi commented Jan 6, 2021

Thomas-Boi commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

Thomas-Boi commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

Thomas-Boi commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

github-actions bot commented Jan 8, 2021

github-actions bot commented Jan 8, 2021

github-actions bot commented Jan 8, 2021

amacado commented Jan 8, 2021

Thomas-Boi commented Jan 8, 2021

github-actions bot commented Jan 8, 2021

github-actions bot commented Jan 8, 2021

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

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

Conversation

Thomas-Boi commented Jan 4, 2021

amacado left a comment

Choose a reason for hiding this comment

Thomas-Boi commented Jan 5, 2021

Thomas-Boi commented Jan 5, 2021

amacado commented Jan 6, 2021

Thomas-Boi commented Jan 6, 2021

Thomas-Boi commented Jan 6, 2021

amacado commented Jan 6, 2021

Thomas-Boi commented Jan 6, 2021

Thomas-Boi commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

Thomas-Boi commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

Thomas-Boi commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

github-actions bot commented Jan 7, 2021

github-actions bot commented Jan 8, 2021

github-actions bot commented Jan 8, 2021

github-actions bot commented Jan 8, 2021

amacado commented Jan 8, 2021

Thomas-Boi commented Jan 8, 2021

github-actions bot commented Jan 8, 2021

github-actions bot commented Jan 8, 2021