Skip to content
This repository has been archived by the owner on Jan 15, 2021. It is now read-only.

WIP: Dynamic contributors #28

Closed
wants to merge 11 commits into from
Closed

WIP: Dynamic contributors #28

wants to merge 11 commits into from

Conversation

bholdt
Copy link
Collaborator

@bholdt bholdt commented Dec 3, 2013

WIP:

@bholdt
Copy link
Collaborator Author

bholdt commented Dec 3, 2013

Still a work in progress. Just making this work visible for anyone who has input.

@bholdt
Copy link
Collaborator Author

bholdt commented Dec 3, 2013

@avanderhoorn if we are going with the new site design, where do you think this will fit?

I will do backend work first (i.e. just knowing who contributes and amount), before doing any front end stuff.

@avanderhoorn
Copy link
Member

Good start mate. Given the changes to the home page/design don't worry too much about the home page - that said if you have a look at the latest update, you will see what I am thinking.

I think its a great idea to focus on the backend services first - since the design is changing and if they are done right we can compose them together in different ways.

As mentioned in #25 the data we should be able to eventually extract from the service is: Reviewers, Committers and Contributors (the rest to come later on). The primary use of the API, will be to build the page mentioned in #25 which has an individual section listing people as mentioned.

For the purposes of the home page, it would be great if one could query the top N contributors (where it would join Reviewers, Committers and Contributors and then take the top N number). All of this doesn't need to be real time and can be cached like the other services, so perf shouldn't be an issue.

Does all this make sense?

@bholdt
Copy link
Collaborator Author

bholdt commented Dec 3, 2013

Perfect

On Tuesday, December 3, 2013, Anthony van der Hoorn wrote:

Good start mate. Given the changes to the home page/design don't worry too
much about the home page - that said if you have a look at the latest
update, you will see what I am thinking.

I think its a great idea to focus on the backend services first - since
the design is changing and if they are done right we can compose them
together in different ways.

As mentioned in #25 https://github.com/Glimpse/Glimpse.Site/issues/25the data we should be able to eventually extract from the service is:
Reviewers, Committers and Contributors (the rest to come later on). The
primary use of the API, will be to build the page mentioned in #25https://github.com/Glimpse/Glimpse.Site/issues/25which has an individual section listing people as mentioned.

For the purposes of the home page, it would be great if one could query
the top N contributors (where it would join Reviewers, Committers and
Contributors and then take the top N number). All of this doesn't need to
be real time and can be cached like the other services, so perf shouldn't
be an issue.

Does all this make sense?


Reply to this email directly or view it on GitHubhttps://github.com//pull/28#issuecomment-29723873
.

@bholdt
Copy link
Collaborator Author

bholdt commented Jan 13, 2014

Back-end process mainly done.
Manually merged this in. Exposed API for new site and old.

@bholdt bholdt closed this Jan 13, 2014
@bholdt bholdt deleted the WIP_DynamicContributors branch January 13, 2014 13:16
@avanderhoorn
Copy link
Member

Just back from holidays and getting back up to speed. To make sure I understand things correctly, this PR setup all the backend APIs we need to be able to write the front end views against?

@bholdt
Copy link
Collaborator Author

bholdt commented Jan 20, 2014

Welcome back! Hope you had a good rest :)

Back end api still needs a bit of tweaking, but you can make use of it
already:
Http://www.getglimpse.com/api/contributors
Or going straight to glimpseContributorService.

@ianoxley is helping us with this too (good to have you on board!). If
either of you needs some extra info I can easily add it in. Still working
on caching, selecting top n contributors and proper people names from
GitHub. This should not affect you all though.

On Monday, January 20, 2014, Anthony van der Hoorn notifications@github.com
wrote:

Just back from holidays and getting back up to speed. To make sure I
understand things correctly, this PR setup all the backend APIs we need to
be able to write the front end views against?


Reply to this email directly or view it on GitHubhttps://github.com//pull/28#issuecomment-32765105
.

@ianoxley
Copy link
Contributor

I've just noticed that the gravatar images returned by /api/contributors from its call to the GitHubAPI are smaller than the contributor images currently shown on the site - they're 80x80 pixels, whereas the current images are slightly larger at 150x150 pixels.

Rendering these smaller images at 150x150 unsuprisingly doesn't look great, but rendering them at 80x80 means that there isn't really enough room to display each contributors name, etc. when the image is flipped over.

A quick hack would be to append '&size=150' onto these gravatar URLs, but that is a quick hack and doesn't feel like the ideal solution.

Has anyone got any better ideas?

@bholdt
Copy link
Collaborator Author

bholdt commented Jan 21, 2014

That's the right way to do it in my opinion though. Where this logic should sit is perhaps the key question though. For me, I see it as a View concern, so whoever is displaying the image should add the size query parameter onto the gravatar URL. (http://en.gravatar.com/site/implement/images/). So, I am happy with that approach and don't see it as a hack.

@avanderhoorn
Copy link
Member

Keep up the great work guys! Looking good.

@nikmd23
Copy link
Member

nikmd23 commented Jan 22, 2014

So, I am happy with that approach and don't see it as a hack.

I'm with @bholdt on that.

@ianoxley
Copy link
Contributor

Ok, no problem. I'll go with that approach :)

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

Successfully merging this pull request may close these issues.

None yet

4 participants