Add cpancover links where appropriate #1128

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
5 participants
@oalders
Member

oalders commented Mar 15, 2014

I think the JSON caching is "good enough" for our purposes right now. I've never used this particular CHI driver in production before, though, so wanted to know if anyone had any concerns with using it. Thought it would be faster than using something on disk and the memory required shouldn't be much. The JSON file is around 43 kb right now, but it should grow as more dists get added to the nightly run.

This is all pretty new (the JSON summary was just implemented yesterday), but it seems this will be updated every 24 hours or so going forward. Also, old reports may not get cached, so we need to be up to date on what is available.

@oalders oalders referenced this pull request Mar 15, 2014

Closed

CPANcover link #732

@monken

This comment has been minimized.

Show comment
Hide comment
@monken

monken Mar 15, 2014

Member

Wouldn't it make more sense to index that data in the api so every consumer of the metacpan api has access to it?

Member

monken commented Mar 15, 2014

Wouldn't it make more sense to index that data in the api so every consumer of the metacpan api has access to it?

@ranguard

This comment has been minimized.

Show comment
Hide comment
@ranguard

ranguard Mar 15, 2014

Member

Yea, via the API would be ideal - though great to have what ever

Member

ranguard commented Mar 15, 2014

Yea, via the API would be ideal - though great to have what ever

@monken

This comment has been minimized.

Show comment
Hide comment
@monken

monken Mar 15, 2014

Member

Well, I disagree. The front end was supposed to be thin wrapper for the api. Now we are adding api logic to it and increase the number of dependencies. For people who contribute to the front end, that is a major pain. I think we should stick to certain standards otherwise we will end up with code smell and an unmaintainable code base.
The request to have that data in the api will come soon anyway. So why not do it right from the beginning? If the cpancover api does not provide the necessary data yet, then the that has to be patched first instead of metacpan working around that.

FWIW, if we decide to keep it in the front end, than at least move it to a model class, where it actually belongs.

Member

monken commented Mar 15, 2014

Well, I disagree. The front end was supposed to be thin wrapper for the api. Now we are adding api logic to it and increase the number of dependencies. For people who contribute to the front end, that is a major pain. I think we should stick to certain standards otherwise we will end up with code smell and an unmaintainable code base.
The request to have that data in the api will come soon anyway. So why not do it right from the beginning? If the cpancover api does not provide the necessary data yet, then the that has to be patched first instead of metacpan working around that.

FWIW, if we decide to keep it in the front end, than at least move it to a model class, where it actually belongs.

@oalders

This comment has been minimized.

Show comment
Hide comment
@oalders

oalders Mar 15, 2014

Member

I think it should be in the API as well, but that's not going to happen this weekend. FWIW, nobody knows exactly how cpancover.com is even going to work. It's all very beta and based on my conversations with @pjcj this weekend there is a lot that's still up in the air. The logic has been abstracted into it's own class, which is essentially a model. The only thing that differentiates it from a model is that it's not in the Model namespace. Thinking about it, I could move it into the Model namespace. That seems reasonable.

I'd like to see this in the API (and soonish), but not until we know how cpancover.com is going to play out. At the very least I'd like to see a stable API on their end before we couple our API with cpancover's API.

This isn't much different from what we're doing with links to GitHub.com except for the fact that we're not slowing down the browser with yet another 3rd party API call.

So, yes, let's put it in the API, but let's not be hasty. The deps aren't unreasonable. There's a VM for contributing to the front end for people who don't want to install a few extra modules. This works right now. It would be reaching to call it a code smell.

Member

oalders commented Mar 15, 2014

I think it should be in the API as well, but that's not going to happen this weekend. FWIW, nobody knows exactly how cpancover.com is even going to work. It's all very beta and based on my conversations with @pjcj this weekend there is a lot that's still up in the air. The logic has been abstracted into it's own class, which is essentially a model. The only thing that differentiates it from a model is that it's not in the Model namespace. Thinking about it, I could move it into the Model namespace. That seems reasonable.

I'd like to see this in the API (and soonish), but not until we know how cpancover.com is going to play out. At the very least I'd like to see a stable API on their end before we couple our API with cpancover's API.

This isn't much different from what we're doing with links to GitHub.com except for the fact that we're not slowing down the browser with yet another 3rd party API call.

So, yes, let's put it in the API, but let's not be hasty. The deps aren't unreasonable. There's a VM for contributing to the front end for people who don't want to install a few extra modules. This works right now. It would be reaching to call it a code smell.

@oalders

This comment has been minimized.

Show comment
Hide comment
@oalders

oalders Mar 18, 2014

Member

Let's not merge this just yet. I've moved the CPANCover logic into a CPAN module, so a model on our end will not be necessary. I just need to run it past @pjcj before I upload anything.

Member

oalders commented Mar 18, 2014

Let's not merge this just yet. I've moved the CPANCover logic into a CPAN module, so a model on our end will not be necessary. I just need to run it past @pjcj before I upload anything.

@oalders

This comment has been minimized.

Show comment
Hide comment
@oalders

oalders Aug 15, 2014

Member

CPANcover is coming along nicely with a lot of coverage, so I won't be merging this as is, but I'm going to leave it open until I can work this stuff into the API. I think I'll also release the CPANCover logic as a module as well, as I don't think @neilbowers has beaten me to it yet.

Member

oalders commented Aug 15, 2014

CPANcover is coming along nicely with a lot of coverage, so I won't be merging this as is, but I'm going to leave it open until I can work this stuff into the API. I think I'll also release the CPANCover logic as a module as well, as I don't think @neilbowers has beaten me to it yet.

@neilb

This comment has been minimized.

Show comment
Hide comment
@neilb

neilb Aug 15, 2014

Contributor

I'vebeen evolving my module in discussion with PJCJ, who has also tweaked the JSON. But I'm not releasing it until his changes go into production - at the moment the module gets the JSON from his staging environment.

Paul thought he may release it this weekend ...

Contributor

neilb commented Aug 15, 2014

I'vebeen evolving my module in discussion with PJCJ, who has also tweaked the JSON. But I'm not releasing it until his changes go into production - at the moment the module gets the JSON from his staging environment.

Paul thought he may release it this weekend ...

@pjcj

This comment has been minimized.

Show comment
Hide comment
@pjcj

pjcj Sep 21, 2014

Contributor

On Thu, Aug 14, 2014 at 10:51:20PM -0700, Neil Bowers wrote:

I'vebeen evolving my module in discussion with PJCJ, who has also
tweaked the JSON. But I'm not releasing it until his changes go into
production - at the moment the module gets the JSON from his staging
environment.

Paul thought he may release it this weekend ...

Yeah, it took a little longer than I had hoped, partly because it took a
month to actually run the coverage on all of CPAN.

But it is done now! And I have made it live. So now all the little,
and not so little, presentation niggles will need to be sorted out.

Feel free to access and link to cpancover.com as appropriate.

Paul Johnson - paul@pjcj.net
http://www.pjcj.net

Contributor

pjcj commented Sep 21, 2014

On Thu, Aug 14, 2014 at 10:51:20PM -0700, Neil Bowers wrote:

I'vebeen evolving my module in discussion with PJCJ, who has also
tweaked the JSON. But I'm not releasing it until his changes go into
production - at the moment the module gets the JSON from his staging
environment.

Paul thought he may release it this weekend ...

Yeah, it took a little longer than I had hoped, partly because it took a
month to actually run the coverage on all of CPAN.

But it is done now! And I have made it live. So now all the little,
and not so little, presentation niggles will need to be sorted out.

Feel free to access and link to cpancover.com as appropriate.

Paul Johnson - paul@pjcj.net
http://www.pjcj.net

@neilb

This comment has been minimized.

Show comment
Hide comment
@neilb

neilb Sep 21, 2014

Contributor

@pjcj: \o/

Contributor

neilb commented Sep 21, 2014

@pjcj: \o/

@oalders oalders added the OPfW 2014 label Oct 7, 2014

@oalders

This comment has been minimized.

Show comment
Hide comment
@oalders

oalders Oct 7, 2014

Member

So, to summarize, this should be moved to CPAN-API/cpan-api. Once that is done we can add cpancover links from metacpan-web.

Member

oalders commented Oct 7, 2014

So, to summarize, this should be moved to CPAN-API/cpan-api. Once that is done we can add cpancover links from metacpan-web.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment