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

Quilkin logo and Quilly the mascot #289

Merged
merged 2 commits into from
Jun 15, 2021
Merged

Quilkin logo and Quilly the mascot #289

merged 2 commits into from
Jun 15, 2021

Conversation

markmandel
Copy link
Contributor

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.

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.
@markmandel markmandel added kind/feature New feature or request area/meta Organisational matters. e.g. Governance, release cycles, etc. labels Jun 2, 2021
@google-cla google-cla bot added the cla: yes label Jun 2, 2021
@quilkin-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: 79b9bac1-67b0-4654-93cf-c19c1b0be0c3

To build this version:

git fetch git@github.com:googleforgames/quilkin.git pull/289/head:pr_289 && git checkout pr_289
cargo build

Copy link
Collaborator

@XAMPPRocky XAMPPRocky left a 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 like assets? 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.

@markmandel
Copy link
Contributor Author

Should this be under docs or under something like assets? I feel like the latter might be more obvious for people to find.

🤔 Hmnnn. I am liking the idea of assets, but was thinking of not wanting to expand the top level folders too much, and I figure we'll primarily be using assets in this folder in our documentation? Some follow on thoughts:

  • docs/assets/logos (if we want to keep things in the docs folder)
  • assets/logos (if we want to go assets as top level, in case we get other asset types)

Thoughts?

Should these AI files be in Git LFS? They're not likely to change much but they do add a lot to the repo.

🤷🏻 it's 4Mb?

@XAMPPRocky
Copy link
Collaborator

🤷🏻 it's 4Mb?

Right, that might not seem like a lot, but relatively just adding it doubles the clone size of repository. The current size-pack for the repository is 4.95 MiB.

@quilkin-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: df36dbe0-bdb5-456e-a50b-fac7591f8a55

To build this version:

git fetch git@github.com:googleforgames/quilkin.git pull/289/head:pr_289 && git checkout pr_289
cargo build

@markmandel
Copy link
Contributor Author

Right, that might not seem like a lot, but relatively just adding it doubles the clone size of repository. The current size-pack for the repository is 4.95 MiB.

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.

@XAMPPRocky
Copy link
Collaborator

Nope, LGTM.

@markmandel markmandel merged commit 582ab80 into main Jun 15, 2021
@markmandel markmandel deleted the docs/logos branch June 15, 2021 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/meta Organisational matters. e.g. Governance, release cycles, etc. cla: yes kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants