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

MetaCPAN does not favor POD over PM #724

Closed
SineSwiper opened this issue Dec 13, 2012 · 24 comments
Closed

MetaCPAN does not favor POD over PM #724

SineSwiper opened this issue Dec 13, 2012 · 24 comments

Comments

@SineSwiper
Copy link

https://metacpan.org/release/BBYRD/DBIx-Class-0.08204_01

The DBIx::Class::InflateColumn link should go to the POD, not the PM file, at least according to the behavior of both perldoc and SCO.

https://github.com/CPAN-API/metacpan-web/blob/master/lib/MetaCPAN/Web/Controller/Module.pm#L28

@monken
Copy link
Contributor

monken commented Dec 13, 2012

when you scroll down you will see under Documentation the link to the pod. The code you linked to is not in effect until the release is not a developer release and the latest release.

@SineSwiper
Copy link
Author

Hmmm, part of the point of the dev release was to test out the display, but yeah, the /module/DBIx... link isn't going to work right now. And I guess I understand the separation of Modules/Docs, even if that will probably cause some confusion...

I guess the big question is: "Does the existing logic re-direct the /module/ link to the POD or PM?"

@oalders
Copy link
Member

oalders commented Dec 13, 2012

@monken For reference:

SineSwiper: [15:56:13] this is sort of a PAUSE question, but here we go
[10:14am] SineSwiper: [15:56:29] I have both POD and PMs with the same "package"
[10:14am] SineSwiper: [15:56:43] I want the POD to take priority
[10:14am] SineSwiper: [15:57:04] https://metacpan.org/module/BBYRD/DBIx-Class-0.08204_01/lib/DBIx/Class/InflateColumn.pm
[10:14am] dipsy: [15:57:05] [ DBIx::Class::InflateColumn - Automatically create references from column data - metacpan.org ]
[10:14am] SineSwiper: [15:57:21] versus https://metacpan.org/module/BBYRD/DBIx-Class-0.08204_01/.generated_pod/DBIx/Class/InflateColumn.pod
[10:14am] dipsy: [15:57:22] [ DBIx::Class::InflateColumn - Automatically create references from column data - metacpan.org ]
[10:14am] SineSwiper: [15:57:49] is this a case of PMLIB ordering? should .generated_pod be the first pick?

@oalders
Copy link
Member

oalders commented Dec 13, 2012

It's a bit confusing that we're using the permalinks in the developer release, but that's not at the root of this particular issue.

@ribasushi
Copy link

I opted to assemble a "classic" dist now, and it still confuses metacpan. Here is the list of issues from a thread of ours[1]

The list of issues comparing:
http://search.cpan.org/~ribasushi/DBIx-Class-0.08204_02/ and
https://metacpan.org/release/RIBASUSHI/DBIx-Class-0.08204_02/

* Documentation section has duplicate entries for stuff existing in
the Modules section (a simple exclusion needs to run before compiling
the list of files)
* If a x.pod and x.pm exist, a web UI must link to the .pod, devrel
or no devrel.
* Metacpan puts DBIx::Class::Optional::Dependencies in the Provides
section for no reason (it is a proper .pm, ought to be in Modules)
* DBIx::Class::Carp is in Modules, even though it is explicitly
no_index-ed

[1] http://lists.scsys.co.uk/pipermail/dbix-class-devel/2012-December/000213.html

@monken
Copy link
Contributor

monken commented Jan 1, 2013

Can you confirm that https://metacpan.org/module/BBYRD/DBIx-Class-0.08204_01/lib/DBIx/Class/InflateColumn.pm is now showing the correct pod?

@SineSwiper
Copy link
Author

Well, technically, yes, the PM link is showing the Pod data in the PM file. However, there is also a file called InflateColumn.pod here:

https://metacpan.org/module/BBYRD/DBIx-Class-0.08204_01/.generated_pod/DBIx/Class/InflateColumn.pod
https://metacpan.org/module/RIBASUSHI/DBIx-Class-0.08204_02/lib/DBIx/Class/InflateColumn.pod

We end up with duplicate links (one of the PM and one for the POD) in the Module/Documentation sections. Based on the _02 distro, it doesn't matter if it's in a different directory or the same one. We want the POD to take priority, since it's the one that has been enhanced with a "INHERITED METHODS" section. Otherwise, it could end up as a source of confusion for users.

The other thing is that we have no way to test the /module/DBIx::Class::InflateColumn link points to the PM or the POD without cutting a production release. I'm not sure if there is an easy way to fix that without cutting an Acme release.

@oalders
Copy link
Member

oalders commented Jan 4, 2013

I don't think we need to use the permalinks in the developer release. They only really make sense if it's the latest version of an authorized production release. So, we should fix that for sure.

@monken
Copy link
Contributor

monken commented Jan 4, 2013

search.cpan.org behaves the same. search.cpan.org/perldoc?DBIx::Class::InflateColumn doesn't point at the dev release and that is kind of the point of having a dev release, isn't it? :)

I agree though, that the documentation should not be duplicated on the release page. However, when do the stable release, both will point at the permalink which resolves to the POD.

@oalders
Copy link
Member

oalders commented Jan 4, 2013

My bad, I thought that the pod links on https://metacpan.org/release/BBYRD/DBIx-Class-0.08204_01 were using permalinks. They're not, so ignore that. :)

@oalders
Copy link
Member

oalders commented Jan 4, 2013

@monken Are you able to join us on IRC briefly?

@monken
Copy link
Contributor

monken commented Jan 4, 2013

unfortunately not, I'm at @work.

@shadowcat-mst
Copy link

I fail to see how dev-release-ness relates.

If a dist has

lib/Foo/Bar.pm # with 'package Foo::Bar;'

and

lib/Foo/Bar.pod

then the .pod file is primary.

See also - the behaviour of perldoc.

@SineSwiper
Copy link
Author

Dev/prod releases matter because /module/Foo::Bar links only points to the latest production release. This really only affects testing. I've already solved that with Acme::Web::PodDisplay, which uses a production version number, and confirms the problem.

@ribasushi
Copy link

It seems that the problem has been solved... at least latest DBIC correctly links where it should. Can one of the devs confirm this has indeed been fixed, and not just a fluke?

@monken
Copy link
Contributor

monken commented Feb 22, 2013

See #724 (comment)
I fixed something back then.

@ribasushi
Copy link

I take my earlier comment back - this issue has not yet been fixed.

The comment in #724 was made on Jan 1st. Several releases since then (0.08205 Jan 23rd, 0.08206 Feb 11, 0.08207 Feb 20) have this problem still (see where the module DBIx::Class::FilterColumn link points to):
https://metacpan.org/release/FREW/DBIx-Class-0.08205/
https://metacpan.org/release/ABRAXXA/DBIx-Class-0.08206/
https://metacpan.org/release/FREW/DBIx-Class-0.08207/

Only the latest release gets it right ( points properly to https://metacpan.org/module/DBIx::Class::FilterColumn ):
https://metacpan.org/release/RIBASUSHI/DBIx-Class-0.08208/

I suspect this has to do with "latest official", which is suboptimal. The linking needs to remain correct within other releases too. For example the public trials have the same problem:
https://metacpan.org/release/RIBASUSHI/DBIx-Class-0.08240-TRIAL/
https://metacpan.org/release/RIBASUSHI/DBIx-Class-0.08241-TRIAL/

@monken
Copy link
Contributor

monken commented Feb 22, 2013

@ribasushi please test

@ribasushi
Copy link

Looks rather excellent now! Thanks for fixing this ;)

@ribasushi
Copy link

I am looking at the fix - it is relatively trivial (almost a one-liner). Wouldn't it be correct to record a list of "pm -> pod" replacements and do not display them in the Documentation section below, the way search.cpan.org does?

@monken
Copy link
Contributor

monken commented Feb 23, 2013

yes

@berekuk
Copy link
Contributor

berekuk commented Mar 6, 2013

I believe a7763af broke some page titles. For example, https://metacpan.org/module/Dancer::Introduction:

- A gentle introduction to Dancer - metacpan.org

When it should be:

Dancer::Introduction - A gentle introduction to Dancer - metacpan.org

module.documentation template var is missing.

@monken
Copy link
Contributor

monken commented Mar 6, 2013

thanks for reporting. I will look into it.

@monken
Copy link
Contributor

monken commented Mar 28, 2013

fixed

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

No branches or pull requests

6 participants