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

github all release + all assets total download count #526

Closed
wants to merge 22 commits into from
Closed

github all release + all assets total download count #526

wants to merge 22 commits into from

Conversation

sagiegurari
Copy link
Contributor

I think the title says it all.
Basically it helps smaller projects with binary assets to show total downloads of all their releases together.

<br>
<a class='photo' href='https://github.com/sagiegurari'>
<img alt='sagiegurari' src='https://avatars.githubusercontent.com/u/8112599?s=80'>
</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to put that in try.html, as index.html gets generated from it.

@espadrine
Copy link
Member

Thanks for contributing!

As I explain in the review, I would prefer to merge the URL for this with /github/downloads/.

Could you squash the commits together?

@sagiegurari
Copy link
Contributor Author

Thanks for the review.
I'll handle those changes tomorrow.

@sagiegurari
Copy link
Contributor Author

I updated based on your comments however, I wasn't 100% sure about github/downloads consolidation, since the URL pattern is different in the current service and wasn't sure if it will catch shorter urls (with less params).
so there are now 2 services for the github/downloads pattern but they have different params amount.
The original:

camp.route(/^\/github\/downloads\/([^\/]+)\/([^\/]+)\/([^\/]+)\/([^\/]+)\.(svg|png|gif|jpg|json)$/,

The added (by me):

camp.route(/^\/github\/downloads\/([^\/]+)\/([^\/]+)\.(svg|png|gif|jpg|json)$/,

There are also now merge conflicts, so do you need to me merge from your master again?
It was after I squashed my last 9 commits into 1.

@sagiegurari
Copy link
Contributor Author

due to all the conflicts I recreated the pull request with an up to date repo.
See #535

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants