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

Include icon for image data #3048

Merged
merged 1 commit into from
Jun 3, 2016
Merged

Conversation

k-nut
Copy link
Contributor

@k-nut k-nut commented May 23, 2016

Fixes #2948, #2910

Proposed fixes:

In order to clean up Pull Requests a little I just took the changes from #2948 and applied the to the lastest master. Of course this still does not include the original vector files as I do no have them.

Features:

  • includes tests covering changes
  • includes updated documentation
  • includes user-visible changes
  • includes API changes
  • includes bugfix for possible backport

@k-nut
Copy link
Contributor Author

k-nut commented May 24, 2016

In #2910 @amercader pointed out that the original files seem to be in this repository: https://github.com/mintcanary/file-types
That repository does not include the new image-type icon though. Maybe @hknochi can tell us where he got it from or provide the original?

@amercader amercader self-assigned this May 24, 2016
@k-nut
Copy link
Contributor Author

k-nut commented May 27, 2016

@amercader I think we should merge this rather soon instead of waiting for the original vector files. Waiting for them led to the last PR becoming conflicting with the current master. I could create a separate issue for also including the vector files?

@hknochi
Copy link

hknochi commented May 27, 2016

@k-nut : i think to add the vector files in the repo won't be the best idea (may these should be in a other/single one)

@k-nut
Copy link
Contributor Author

k-nut commented May 27, 2016

Yes. We might want to store them in a separate repo. But at least having something like a link to the original somewhere in the ckan project could prove useful already.

@hknochi
Copy link

hknochi commented May 27, 2016

okay i'll add the source . you can move them to a custom path afterwards

@hknochi
Copy link

hknochi commented May 27, 2016

please take a look here: https://drive.eccenca.com/d/73541a96f2/

@k-nut
Copy link
Contributor Author

k-nut commented May 27, 2016

@hknochi cool, thank you! How are the icons licensed?

@k-nut
Copy link
Contributor Author

k-nut commented May 27, 2016

@amercader do you think we should create a new repository for design assets like this? Or is there something already where we could put this? Or would you just include it in this repository?

@seebi
Copy link

seebi commented May 27, 2016

Fromm eccenca's POV this should not be decided by us but by @smth who originally pushed these files (mintcanary/file-types@1740cd6). The work of @hknochi can be licensed under any terms suitable for you.

@smth
Copy link
Contributor

smth commented May 30, 2016

I just added a license here.

@k-nut
Copy link
Contributor Author

k-nut commented Jun 1, 2016

Created a first pull request in the new ckan-assets repository. Once that is merged this PR should also be good to merge.

@amercader amercader merged commit a8d4ab7 into ckan:master Jun 3, 2016
@k-nut k-nut deleted the 2948-icon-for-image-files branch June 6, 2016 11:33
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.

5 participants