-
Notifications
You must be signed in to change notification settings - Fork 98
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
Quilkin logo and Quilly the mascot #289
Conversation
Includes the original AI files (they also open in Inkscape for editing), and some PNG exports of both the white log and Quilly on transparent backgrounds. Updated the README to also include both.
Build Succeeded 🥳 Build Id: 79b9bac1-67b0-4654-93cf-c19c1b0be0c3 To build this version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! There are a couple of things, but I'm fine with this as-is.
- Should this be under
docs
or under something likeassets
? I feel like the latter might be more obvious for people to find. - Should these AI files be in Git LFS? They're not likely to change much but they do add a lot to the repo.
🤔 Hmnnn. I am liking the idea of
Thoughts?
🤷🏻 it's 4Mb? |
Right, that might not seem like a lot, but relatively just adding it doubles the clone size of repository. The current |
Build Succeeded 🥳 Build Id: df36dbe0-bdb5-456e-a50b-fac7591f8a55 To build this version:
|
Seems like more trouble than it's worth to setup and maintain Git LFS for a few MB? As well as the extra complexity for maintainers? We can always revisit if we end up with lot more binary files. Sounds like there aren't many strong opinions about the changes above - so if not, I'll merge as is. |
Nope, LGTM. |
Includes the original AI files (they also open in Inkscape for editing), and some PNG exports of both the white log and Quilly on transparent backgrounds.
Updated the README to also include both.