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

feat(languages): use github language names and add new icons #2270

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ReenigneArcher
Copy link

@ReenigneArcher ReenigneArcher commented Sep 15, 2024

Double check these details before you open a PR

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

Features

This PR adds language icons and alias that match the naming GitHub uses in the GitHub API. GitHub uses "linguist" for the languages API. Their list of languages can be found here: https://github.com/github-linguist/linguist/blob/master/lib/linguist/languages.yml

The advantage of this is it makes it possible to use icons directly, without creating a custom mapping.

  • Many new icons, from https://github.com/LizardByte/uno/tree/master/gh-pages-template/language-icons
  • powershell icon updated
  • current nginx icon moved to wordmark icon and added new original icon
  • jest-plain renamed to jest-original
  • fixed graphql "plain" icons -> original
  • re-ordered a couple of out of order icons, like windows8 and reactrouter
  • some new altnames added (taken from the linguist list)
  • some tags added or changed from existing icons (taken from the linguist list)

This PR closes NONE

Notes

I understand that typically new icons are probably submitted one per PR; however since this is somewhat of a new feature (names to match github API), I'd really rather avoid 230 PRs.

The colors for new icons were selected using a color picker. If there was a more prominent color, I selected that, otherwise it may be a random color in some cases (where an icon may have many colors).

I have a question on "graphql". There is no "original" icon, and no alias to it. Is that correct? Edit: I updated the graphql icons based on the comment below.

@ReenigneArcher ReenigneArcher force-pushed the feat/languages/use-github-language-names-and-add-new-icons branch 4 times, most recently from f65dd52 to 011bf7c Compare September 15, 2024 02:21
@Snailedlt
Copy link
Collaborator

Snailedlt commented Sep 15, 2024

@ReenigneArcher all icons should have an original version, that includes graphql. So that's not intended

Edit: The graphql icons should have been named "graphql-original" and "graphql-original-wordmark", and had aliases to "plain" and "plain-wordmark" respectively.

@ReenigneArcher ReenigneArcher force-pushed the feat/languages/use-github-language-names-and-add-new-icons branch 17 times, most recently from bc73c6f to 2b6372c Compare September 16, 2024 20:12
@ReenigneArcher ReenigneArcher marked this pull request as ready for review September 16, 2024 20:16
@ReenigneArcher
Copy link
Author

This PR is ready for review.

@ReenigneArcher ReenigneArcher force-pushed the feat/languages/use-github-language-names-and-add-new-icons branch from 2b6372c to 6f71aa6 Compare September 29, 2024 02:11
@ReenigneArcher ReenigneArcher force-pushed the feat/languages/use-github-language-names-and-add-new-icons branch from 6f71aa6 to a0c3b85 Compare October 3, 2024 17:40
@ReenigneArcher
Copy link
Author

ReenigneArcher commented Oct 3, 2024

I rebased this again. Regarding the recent awk icon, I kept my icon since I followed the guidelines here (https://github.com/devicons/devicon/wiki/SVG-Standards) and the PR that was merged did not (both padding and view box size were not followed).

I'd like to request a review as I do not want to rebase this repeatedly. I understand it's a really big PR, so please let me know if there's anything I can do to help the process.

@Snailedlt
Copy link
Collaborator

@ReenigneArcher I sadly don't have much time to review this for some time.

A few things though:

  1. You don't need to rebase the PR, we actually prefer merging since that makes it easier to see what changed between each commit, making it easier to review. You can also ignore the PR out-of-date warnings until the PR is reviewed. We usually just merge the latest develop changes once the PR is ready to be merged.

  2. We actually need a PR for each updated or new icon. This is because our automated workflow uses the PR name to create the font before a new release. So if the icons change or the "svg" or "font" fields in devicon.json change it will need to be done in a separate PR.
    If we have everything in a big PR we'd have to create the font manually which would be quite laborious. This is something I hope we can improve in the future, but sadly I don't have time to do it myself.

  3. If you change the name of an icon that is a breaking change, therefore I suggest you bundle the icons where only the name changes into a separate PR. That way we can still release the other stuff before we change the names.

Sorry about the late reply on this, but I hope this was useful info for you.

Don't hesitate to reach out if you need anything, whether it be advice, help or you have any questions

@ReenigneArcher
Copy link
Author

For point 1, it had to be rebased because there were conflicts. Maybe breaking out the json file into multiple files would be helpful in these cases... Actually the icons in separated folders would be as well. Like an A-Z parent directory.

Regarding point 2, do you think that can be improved anytime soon? I don't have time to create 149 PRs, probably no one does, lol.

@ReenigneArcher
Copy link
Author

Regarding point 2 (again)... It looks like these are areas of interest?

Would you accept a PR allowing this to run over all the changed or new icons?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants