-
Notifications
You must be signed in to change notification settings - Fork 4
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 logo and benchmarks to README #60
Conversation
Codecov ReportPatch and project coverage have no change.
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
=======================================
Coverage 71.62% 71.62%
=======================================
Files 89 89
Lines 3461 3461
Branches 642 642
=======================================
Hits 2479 2479
Misses 783 783
Partials 199 199 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
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.
This one also looks great, Erik! :)
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.
It's a small thing, but the last letter in "graphblas" is a bit cropped out. I think it still works, though, so not a serious issue.
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.
Really? I don't think I see that. Can you take a screenshot of what you are seeing?
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.
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.
And now that I'm looking at it again, the last s in algorithms is also gone... Interesting.
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.
I tried playing around with zoom to see if that would make a diff, but nope.
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.
I see this now (on my macbook); it renders fine for me on Windows where I created it. Thanks for letting me know!
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.
Okay, I think the logo should be fixed now if you'd like to take a look to verify.
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.
To recap testing on my end: looks good on MacOS and Windows, looks truncated on Ubuntu.
nope looks the same
…On Sat, 22 Apr 2023 at 03:39, Erik Welch ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
On docs/_static/img/logo-name-medium.svg
<#60 (comment)>
:
Okay, I think the logo should be fixed now if you'd like to take a look to
verify.
—
Reply to this email directly, view it on GitHub
<#60 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE2FWESMMOA7WNUKCUXOEUTXCL5A3ANCNFSM6AAAAAAW7UPWYM>
.
You are receiving this because your review was requested.Message ID:
***@***.***
com>
|
Dope! Thanks for checking. Jim and I have learned that the font looks very different on different systems (Windows, Mac, Android, etc). So, I think @jim22k is going to convert all the svg logos to PNGs (Jim, don't forget to minimize the PNGs--such as from https://compresspng.com/ and https://tinypng.com/ --to improve page load times!). |
I switched from svg to png. This bumps the logo size from 1k to 35k, but I think it's worth it to avoid the need to grab fonts from the local machine, which leads to different looks and widths. PNG will always be consistent. |
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.
Looks good! Interesting the file size is so large (35K), but small enough to not be an issue.
Thanks for converting the svg font to paths @jim22k. I minified the svg (that you converted the font into paths) with https://vecta.io/nano and deleted the png, b/c the svg looks better. I think this is good to go in 🎉 Now let's do the same for the svgs in |
This is a companion PR to python-graphblas/python-graphblas#432
I copied the logo from that PR and manually modified it.