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

Added Django icon #267

Closed
wants to merge 1 commit into from
Closed

Conversation

batisteo
Copy link

@batisteo
Copy link
Author

@PKief Anything else needed on this PR?

screenshot from 2018-06-22 10-52-49

@PKief
Copy link
Owner

PKief commented Jun 22, 2018

The only problem I see is that you selected the following extensions:

fileExtensions: ['html', 'txt']

They will override the icons for html and txt at all.
So all people that are using index.html will see the django icon.

The language ids are good enough here.

Please remove this row:

{ name: 'django', fileExtensions: ['html', 'txt'] }

@batisteo
Copy link
Author

Thanks for pointing that out. Done!

@PKief PKief closed this in dd7f222 Jun 22, 2018
@PKief
Copy link
Owner

PKief commented Jun 22, 2018

Alright, I reviewed the icon too. I adjusted the color to Green 600 because it's slightly lighter and matches better to the other icons. Additionally I improved the size a little bit.

I also found out that some guys are using the file extension djt for django so I added it, too.

django icon

@PKief
Copy link
Owner

PKief commented Jun 22, 2018

Thank you very much for your pull request 👍

@batisteo
Copy link
Author

I also found out that some guys are using the file extension djt for django

Apparently. I never met one of them yet after 8 years in the field 🙂
It doesn’t hurt.

Thanks for the review!

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.

None yet

2 participants