-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Added gem downloads count #212
Conversation
also i think that this application needs to send headers for Pragma: no-cache (used by HTTP 1.0 and IE browsers) I can add them myself, but i wanted to ask you if that would be ok before i do it. Please let me know . Thanks |
Thanks a lot! Other download-related URLs on Shields.io are of the form We're sending no-cache on line 142, along with no-store and must-revalidate. When I tested it, those were enough to prevent GitHub from caching them too strongly. They still used camo, but the images were updated every time. I'd have to check whether that is no longer the case, unless you want to do it of course! If that's the case, we should open a new issue. |
About the rubygems apirubygems api doesn't provide information about downloads per month yet or for any other time frame, maybe will be in the future available this information. Right now the api provides three urls for getting download count
If you consider i should use this other two api methods i can do it , just let me know. I am using all three of them in my application. I just didn't considered them necessary here. About the cache problemI saw that you're sending the Cache-Control header, but when i developed my application, using just the Cache-Control header didn't helped, not even if i added a Etag header and a Last-Modified header, And if you read the documentation from Fastly, you will see that there is a prioritization in how the headers are used. They also use this header
but this can be replaced by
The difference is that the second one is not stripped and is respected by Fastly caches. Problem is not all browsers know about "Cache-Control : s-maxage = (time in seconds) " directive. So it is said in the documentation that it is better to use also
which works better with most of the browsers. So what did worked for me was this:
I am not sure if this was a problem with Sinatra only, but for me only this worked. That's why i considered that i should let you know about it. If you consider there are not necessary, it's fine with me. You can also read this github/markup#224, and see that a lot of people reported this problem. |
Update: Searching again through the api seems there is possible to get the downloads count for a time frame by day using this api url
or this:
The second one returns also the downloads count by day, starting from today - 89.days until today The result looks like this :
So i could get the downloads count for a time frame if i add this numbers . But i am not sure this actually works, from what i can see it returns zero for all dates, which is impossible , especially if we think about the gem "rails" which i think was downloaded at least one time in the past 90 days And this api methods are not listed on their public page http://guides.rubygems.org/rubygems-org-api/ , I only discovered them by loooking at source code, so probably there are not supported |
Wow, that is outstandingly exhaustive information! Ideally, all badges should have the same meaning of "total downloads". Unfortunately, this might not be achievable, given the APIs at hand. For the record, Packagist's total downloads does not seem version-dependent, nor does NuGet's (Chocolatey's). If we should have two "total download" badges with differing meanings, that difference should be made obvious in the badge. Currently, for instance, we have I applaud your digging through code to find the date-range API. Is there a rubygems.org contributor we can ping about future hopes of fixes for this API? |
Honestly, I agree with the total download badge to look like this
Because i think having in the same sentence the words "version" and "total" can be confusing . I think this way is better, but is just my opinion. I am ok however with any decision that you make , just let me know. Also i have one more question. You previously suggested that the url for getting the total downloads should be And about your last question...if there is a contributor of rubygems.org that we can ping about this ...i honestly don't know, however this problem with the date range api is listed in their issue list rubygems/rubygems.org#616 , and from what i can see they decided to remove the functionality , that's why probably there are not listed on their public documentation. I can submit a new issue there, but since they decided to remove it , i am not even sure if this can be considered an issue. |
You're probably right! Having nothing, however, makes the exact meaning of the badge unclear. Maybe "(latest version)"? Then again, anyone can change the first label for the badge using As for the URL, I think the least risky for future compatibility is By the way, I made a new issue rubygems/gems#22 to ask for a way to get monthly downloads from the API. |
Yes, i agree . Having nothing can be confusing also but my point was not about the label , but about the right side of the badge, the text after the number of downloads. I only suggested to have nothing after that number because looking at other sites like http://www.gemetric.me/ they are doing it in the same way. But was only a suggestion. But i like your suggestion also. Something like this could look nice:
and if someone will have a better idea, they can submit a issue afterwards and that text can be adjusted accordingly. So i guess i am going to go with your suggestion. Seems rather more clear. Thanks for having the time to answer my questions. I really appreciate your help. I will do the necesary changes tomorrow and submit this new changes to this PR. |
I made the requested changes, please let me know if it is ok. I added my mistake the headers for Pragma and Expires, but i deleted them afterwards. Sorry for that. Thanks for your help |
This looks excellent! I noticed there was a I'm really eager to merge this! |
forgot to edit that line. Fixed it now . |
Thanks a lot! It is live now! |
thanks : ) |
I'm researching the no-cache stuff. Apparently, sending |
you could test with older versions of IE that uses HTTP 1.0. Using HTTP 1.1 , it's not necessary. Thing is older versions of IE have by default enabled only HTTP 1.0 . This header is only to prevent the browser for caching the badge. But i agree newer versions don't need this anymore. but it's really your decision. And is not so important so i think we can just overlook this if that is your decision. |
This was requested in previous pull request : #211
Please let me know if this is ok!. I tested and seems to be working ok . Thank you