-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Smaller logo #3901
Smaller logo #3901
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3901 +/- ##
==========================================
+ Coverage 89.17% 89.32% +0.15%
==========================================
Files 15 15
Lines 1940 1940
==========================================
+ Hits 1730 1733 +3
+ Misses 210 207 -3
Continue to review full report at Codecov.
|
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.
Thanks @nedbat! Looks great!
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.
Actually one smaller note.
README.rst
Outdated
@@ -50,7 +50,7 @@ Behold, the power of Requests: | |||
|
|||
See `the similar code, sans Requests <https://gist.github.com/973705>`_. | |||
|
|||
.. image:: http://docs.python-requests.org/en/master/_static/requests-sidebar.png | |||
.. image:: https://raw.githubusercontent.com/nedbat/requests/master/docs/_static/requests-logo-small.png | |||
:target: http://docs.python-requests.org/ |
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 should probably be pointed into our copy of the repo, not yours. 😉
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 know, i mentioned that on the pull request. :) If I made it point to kenneth's now, I wouldn't be able to see it in my repo, and neither would you :)
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 can change the URL now in the PR if you like, or fix it afterwards.
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.
Let's fix it now. I don't like adding extra commits if I can avoid it. 😆
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.
Done
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.
Woo!
Nice first commit, thanks @nedbat! |
The logo as-is is too large, it won't even fit on my screen!
I made a smaller version for use in the README.rst, because GitHub has neutered some useful features and doesn't care: github/markup#295
NOTE: the image path is to my fork. Once merged, the path should be changed to refer to the main repo.