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

Make example scripts distributed with a release more prominent #741

Merged
merged 8 commits into from Mar 20, 2013

Conversation

tobyink
Copy link
Contributor

@tobyink tobyink commented Jan 9, 2013

Many authors distribute example scripts with their releases.

Here is a small patch to include a list of examples on the release info page.

@monken
Copy link
Contributor

monken commented Jan 10, 2013

Have you tried extending https://github.com/CPAN-API/metacpan-web/blob/master/lib/MetaCPAN/Web/Model/API/Release.pm#L247 ? that would safe us a roundtrip to the api and I don't see a reason why that shouldn't work.
You can then extract the examples again from the files returned and render them differently.

@tobyink
Copy link
Contributor Author

tobyink commented Jan 10, 2013

I'll investigate that option.

@tobyink
Copy link
Contributor Author

tobyink commented Jan 10, 2013

Currently failing t/controller/diff.t because of this broken page:

Diff page is missing some link text

Not entirely sure how my edits could have caused this breakage. I've not touched the Diff model, controller or template. It may just be a temporary issue with the MetaCPAN API?

@rwstauner
Copy link
Contributor

I have some commits to push to the api later that might fix that diff issue

@monken
Copy link
Contributor

monken commented Jan 10, 2013

@rwstauner do you know when/how that regression was introduced? It certainly worked at some point.

@rwstauner
Copy link
Contributor

I found an issue on my dev machine where the diff paths were getting truncated and I think the difference (b/t working and not) is a difference of environment. I believe I have fixed that though (locally).
As far as I can tell the live api is still working normally, though, so the two things may not be related.
We'll see.

@rwstauner
Copy link
Contributor

@rwstauner
Copy link
Contributor

Nevermind. I deployed the api changes and https://metacpan.org/diff/file/?target=DOY/Moose-2.0202/lib/Moose.pm&source=DOY/Moose-2.0201/lib/Moose.pm is still missing the file names.

@rwstauner
Copy link
Contributor

@tobyink I fixed the api; the diff should display correctly for you now. Thanks.

@rwstauner
Copy link
Contributor

I also fixed the test in this repo.

@tobyink
Copy link
Contributor Author

tobyink commented Jan 22, 2013

I've pulled in all kinds of upstream stuff, and it now passes all tests.

oalders added a commit that referenced this pull request Mar 20, 2013
Make example scripts distributed with a release more prominent
@oalders oalders merged commit 9630354 into metacpan:master Mar 20, 2013
@oalders
Copy link
Member

oalders commented Mar 20, 2013

Thanks very much for this. My apologies that we lost track of it. Please don't let that discourage any future contributions. This is a very helpful addition. 👍

@karenetheridge
Copy link
Contributor

thank you very much!

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.

None yet

5 participants