Skip to content

[BUG] check-bot check -original icons even when they won't be added to icomoon #1325

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

Open
1 task done
BenSouchet opened this issue Jul 27, 2022 · 2 comments · Fixed by #1491
Open
1 task done

[BUG] check-bot check -original icons even when they won't be added to icomoon #1325

BenSouchet opened this issue Jul 27, 2022 · 2 comments · Fixed by #1491
Assignees
Labels
bug Use this label for pointing out bugs devops Use this label for devops related enhancements

Comments

@BenSouchet
Copy link
Contributor

I have searched through the issues and didn't find my problem.

  • Confirm

Bug description

Currently check-bot perform check operations related to icomoon specifications like the "no usage of stroke(s) attribute(s) in paths" even when the icons won't be included in the devicon font.

Possible fixes or solutions

When an icon -plain.svg exist in a PR, don't check -original.svg the icomoon rules.
And also when an icon -plain-wordmark.svg exist in a PR, don't check -original-wordmark.svg the icomoon rules.

Additional information

No response

@BenSouchet BenSouchet added the bug Use this label for pointing out bugs label Jul 27, 2022
@BenSouchet
Copy link
Contributor Author

The issue can be seen in this PR #1323

@Snailedlt Snailedlt added the devops Use this label for devops related enhancements label Oct 8, 2022
@Snailedlt
Copy link
Collaborator

Snailedlt commented Oct 30, 2022

The issue can also be seen in this PR: #1488

I think we can simplify the fix for this issue by simply checking the font attribute in devicon.json, which tells us whether or not the icon will be uploaded to icomoon. So pretty much, if the icon exists inside font, check the stroke property, otherwise it can be skipped.

Should only need to modify these lines then:

for child in tree.iter():
if child.get("stroke") != None:
err_msg.append("- SVG contains `stroke` property. This will get ignored by Icomoon. Please convert them to fills.")
break

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Use this label for pointing out bugs devops Use this label for devops related enhancements
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

2 participants