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

fix check svg icons workflow #508

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

amacado
Copy link
Member

@amacado amacado commented Feb 21, 2021

add check for directory /icons when workflow is executed (fixes #505)

@amacado amacado added devops Use this label for devops related enhancements enhancement labels Feb 21, 2021
@github-actions
Copy link
Contributor

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.

Please let my maintainers know of the issues. They will take a look at my work and try to resolve the problem. Until then, please hang tight and sorry for the inconvenience.

Cheers,
SVG-Checker Bot 😄

@amacado amacado linked an issue Feb 21, 2021 that may be closed by this pull request
@amacado
Copy link
Member Author

amacado commented Feb 21, 2021

@Thomas-Boi I came up with a solution for #505, but there might be a more elegant solution? I'm not very familiar with python.

@amacado amacado mentioned this pull request Feb 21, 2021
4 tasks
Thomas-Boi
Thomas-Boi previously approved these changes Feb 22, 2021
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.

Everything seems good for me.

@Thomas-Boi Thomas-Boi dismissed their stale review February 22, 2021 02:20

Wait, let me double check. If I recall, the path name might not start with /icons

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.

Nope, false alarm. It seems like the path is relative and not absolute as I feared. In conclusion, we are good to go.

@Panquesito7
Copy link
Member

Why did the bot report an issue? Is it because there are no .svg's being modified in this branch?

@Thomas-Boi
Copy link
Member

Why did the bot report an issue? Is it because there are no .svg's being modified in this branch?

It was because there is an .svg file in the /fonts that it also checks. While the .svg in the /icons need to adhere to our standards, the one in the /fonts don't need to => The bot doesn't know this and just report what it finds.

@amacado
Copy link
Member Author

amacado commented Feb 22, 2021

Why did the bot report an issue? Is it because there are no .svg's being modified in this branch?

You are right! In this pull request the bot reported an error because there are no new/modified svg's in this branch. We could fix this. I will draft this pull request and see what I can do.

image
https://github.com/devicons/devicon/runs/1948022248?check_suite_focus=true

@Thomas-Boi refers to the other bot comments on the pull requests where the bot reported an error with devicon.svg (as I described the problem in #505)

Update: This is fixed/continued in #509 (comment)

Fix the above issue so it don't post the wrong message
Post no message if there's no SVG to check (it's getting annoying 🙃)

@amacado amacado marked this pull request as draft February 22, 2021 11:39
@amacado amacado marked this pull request as ready for review February 22, 2021 11:48
@amacado amacado merged commit 8bc3da3 into develop Feb 22, 2021
@amacado amacado deleted the amacado/feature/505-fix-bug-check-svg-workflow branch February 22, 2021 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops Use this label for devops related enhancements enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workflow "check the SVGs' quality" reports error on "devicon.svg"
3 participants