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

Add GMC icon #11265

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Conversation

DevangGarg
Copy link

@DevangGarg DevangGarg commented Jun 18, 2024

Issue: closes #11248.

Popularity metric:

Checklist

  • I updated the JSON data in _data/simple-icons.json
  • I optimized the icon with SVGO or SVGOMG
  • The SVG viewbox is 0 0 24 24

Description

@simple-icons simple-icons bot added docs Issues for improving or updating package documentation new icon Issues or pull requests for adding a new icon labels Jun 18, 2024
@adamrusted adamrusted changed the title Added GMC icon successfully. Add GMC icon Jun 18, 2024
Copy link
Member

@adamrusted adamrusted left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this @DevangGarg - there are a couple of points below, and I'd appreciate you completing the PR template in your post above to give us proper metrics for measuring popularity, etc.

slugs.md Outdated Show resolved Hide resolved
icons/gmc.svg Outdated Show resolved Hide resolved
_data/simple-icons.json Outdated Show resolved Hide resolved
@adamrusted adamrusted added incomplete Issue or pull request where the required information is either incomplete or missing altogether permission required Permission should be sought out from the trademark owner before including this icon in our project. labels Jun 18, 2024
@anhthang

This comment was marked as resolved.

Copy link
Member

Choose a reason for hiding this comment

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

This icon does not at all match the one from the URL listed as source.

{
"title": "GMC",
"hex": "E21836",
"source": "https://www.gmc.com/content/dam/gmc/na/us/english/primary-nav-icons/GMC-153x28.svg"
Copy link
Member

Choose a reason for hiding this comment

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

As per https://github.com/simple-icons/simple-icons/blob/develop/CONTRIBUTING.md#source-guidelines we should avoid linking directly to the SVG files themselves and rather link to the page that has the SVG displayed on it.

@uncenter uncenter removed the docs Issues for improving or updating package documentation label Jun 18, 2024
@jrhaberland
Copy link
Contributor

Depending on the answer from General Motors we may need to remove the Chevrolet, Cadillac, and GM Icon

@uncenter
Copy link
Member

Depending on the answer from General Motors we may need to remove the Chevrolet, Cadillac, and GM Icon

Correct. And #11247 would have to be closed.

@adamrusted
Copy link
Member

Has someone asked for permission on this one yet?
Assuming not, can you please reach out to the brand @DevangGarg? You can find guidelines here.

@DevangGarg
Copy link
Author

Has someone asked for permission on this one yet? Assuming not, can you please reach out to the brand @DevangGarg? You can find guidelines here.

I think that General motors has said to remove the icon. Do I need to continue with this task?

@adamrusted
Copy link
Member

I think that General motors has said to remove the icon. Do I need to continue with this task?

Where have they said this? Only thing I can see on this thread is me sharing the link to their terms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incomplete Issue or pull request where the required information is either incomplete or missing altogether new icon Issues or pull requests for adding a new icon permission required Permission should be sought out from the trademark owner before including this icon in our project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request: GMC
5 participants