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

Feature/update check icon pr py #1406

Merged
merged 8 commits into from
Mar 17, 2023

Conversation

Snailedlt
Copy link
Collaborator

@Snailedlt Snailedlt commented Oct 2, 2022

Double check these details before you open a PR

  • PR does not match another non-stale PR currently opened

Features

  1. Check if name attribute exists and is of the correct type (string)
  2. Check if altnames attribute exists, is of correct type (array of strings) and does not contain name
  3. Add extra checks to aliases to ensure they contain base and `alias, and that they both match the regexp: (original|plain|line)(-wordmark)?

This PR closes NONE

Notes

@Snailedlt Snailedlt added enhancement devops Use this label for devops related enhancements labels Oct 2, 2022
@Snailedlt Snailedlt requested review from Thomas-Boi, a team, amacado and Panquesito7 and removed request for a team October 2, 2022 00:33
Thomas-Boi
Thomas-Boi previously approved these changes Oct 2, 2022
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.

Hey @Snailedlt,

The PR looks great 👍 . The checks make sense to me and it seems like we now have a altname field in the object. This should be a great feature for the checking script. I'll approve this PR. However, I'm not a maintainer anymore so please double check with the others just in case.

Good luck with future PRs. I think you'll be a great member for the Devicon maintainer team 😄

@Snailedlt
Copy link
Collaborator Author

@Panquesito7 please check this out when you have time :)
Should be merged ASAP since it will make reviews easier and quicker

@Snailedlt Snailedlt added the hacktoberfest-accepted Accepted to be counted towards Hacktoberfest label Oct 31, 2022
Copy link
Contributor

@lunatic-fox lunatic-fox left a comment

Choose a reason for hiding this comment

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

I've reviewed it and forgot to finish. 😑

.github/scripts/check_icon_pr.py Outdated Show resolved Hide resolved
.github/scripts/check_icon_pr.py Outdated Show resolved Hide resolved
Apply suggestions from code review. Thanks @lunatic-fox

Co-authored-by: Josélio Júnior <[email protected]>
@Snailedlt Snailedlt requested a review from lunatic-fox January 9, 2023 18:22
.github/scripts/check_icon_pr.py Outdated Show resolved Hide resolved
.github/scripts/check_icon_pr.py Outdated Show resolved Hide resolved
Apply suggestions from code review

Co-authored-by: David Leal <[email protected]>
@Snailedlt Snailedlt requested a review from Panquesito7 February 1, 2023 08:52
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Great work! Thanks. 🚀

@Snailedlt
Copy link
Collaborator Author

@lunatic-fox if this looks okey to you I can go ahead and merge it to develop :)

@Snailedlt Snailedlt requested a review from ConX March 16, 2023 08:24
@Snailedlt Snailedlt merged commit 67c06f4 into devicons:develop Mar 17, 2023
@Snailedlt Snailedlt mentioned this pull request Feb 5, 2024
GCHQDeveloper926 pushed a commit to GCHQDeveloper926/devicon that referenced this pull request Dec 20, 2024
* add check for 'name' attribute

* add check for 'altnames' attribute

* add extra check for content of aliases field in devicon.json

* Change from single quotes to escaped double quotes

Apply suggestions from code review. Thanks @lunatic-fox

Co-authored-by: Josélio Júnior <[email protected]>

* Replace ' with \"

Apply suggestions from code review

Co-authored-by: David Leal <[email protected]>

---------

Co-authored-by: Josélio Júnior <[email protected]>
Co-authored-by: David Leal <[email protected]>
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 hacktoberfest-accepted Accepted to be counted towards Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants