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/sort devicon json and add sort check #1405

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. Sort and re-indent devicon.json
  2. Add check to check_icon_pr.py to see if devicon.json is still sorted

This PR closes #1337, #1327

Notes

Please check both if the functionality looks sound, and if the error messages can be improved

@Snailedlt Snailedlt added enhancement devops Use this label for devops related enhancements labels Oct 2, 2022
@Snailedlt Snailedlt requested review from Thomas-Boi, Panquesito7, a team and amacado and removed request for a team and Panquesito7 October 2, 2022 00:18
@Snailedlt Snailedlt added the hacktoberfest-accepted Accepted to be counted towards Hacktoberfest label Oct 31, 2022
@Snailedlt Snailedlt marked this pull request as draft November 19, 2022 13:44
@Snailedlt
Copy link
Collaborator Author

The sorting of devicon.json in this PR needs to be redone because devicon.json has been updated since it was last sorted

Sort `devicon.json`  of commit `77ff589df1fcffde36d9f3239d658046663d5588` from `develop` branch.
@lunatic-fox
Copy link
Contributor

Code used to sort devicon.json

import json
path = 'devicon.json'

with open(path) as file:
  file: list[dict] = json.load(file)
  file.sort(key=lambda e: e['name'])
  file = json.dumps(file, indent=4) + '\n'

with open(path, 'w') as output_file:
  output_file.write(file)

@Snailedlt
Copy link
Collaborator Author

@lunatic-fox did you check if the check-icon script deemed it as sorted?

@lunatic-fox
Copy link
Contributor

@lunatic-fox did you check if the check-icon script deemed it as sorted?

Not yet, I'll do later some tests in a private repo.

.github/scripts/check_icon_pr.py Outdated Show resolved Hide resolved
.github/scripts/check_icon_pr.py Outdated Show resolved Hide resolved
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.

It'll work properly after fixing the reference error.

Preview

preview

.github/scripts/check_icon_pr.py Outdated Show resolved Hide resolved
.github/scripts/check_icon_pr.py Outdated Show resolved Hide resolved
Co-authored-by: Josélio Júnior <[email protected]>
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.

  • devicon.json is up-to-date and sorted with its latest version: eb35d73
  • Conflicts are resolved.

Looking good! ✔
Thank you for this great feature! 🚀

@Snailedlt Snailedlt marked this pull request as ready for review December 11, 2022 21:30
@Snailedlt Snailedlt merged commit 3d20d58 into devicons:develop Dec 11, 2022
@Snailedlt Snailedlt mentioned this pull request Feb 5, 2024
GCHQDeveloper926 pushed a commit to GCHQDeveloper926/devicon that referenced this pull request Dec 20, 2024
* indent and sort devicon.json

* add check to see if devicon.json is sorted

* Update devicon.json

Sort `devicon.json`  of commit `1252ce95619756b5c237c72c605288e5eadc118f` from `develop` branch.

* Apply suggestions from code review

* Apply suggestions from code review

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

* Update `devicon.json` with 5460505

Co-authored-by: unknown <[email protected]>
Co-authored-by: Josélio Júnior <[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.

2 participants