search

libwww-perl / URI

The Perl URI module
https://metacpan.org/pod/URI
Other
41 stars 48 forks source link

Check that scheme-specific class inherits from URI #51

Closed nfg closed 6 years ago

nfg commented 6 years ago

Hello!

Here's a stab at #50. I wasn't sure what other tests should be added under tel.t, but this was enough to prove that the bug went away. Let me know if there's anything that should change.

Cheers!

Fixes #50

oalders commented 6 years ago

Hi Nigel,

Thanks so much for this! Could you add a comment in the code that describes the specific problem that we're fixing? I think that would be helpful for future reference.

nfg commented 6 years ago

Done! Is there anything else I can do?

oalders commented 6 years ago

Thanks! I think we could specifically mention URI::tel in the comments just to be 100% clear what is going on. Other than that, I'll just try to get one other person to review this before we merge.

skaji commented 6 years ago

FWIW this PR breaks https://metacpan.org/source/TBR/URI-tel-0.03/t/20-uri.t

skaji commented 6 years ago

And this also breaks https://metacpan.org/release/URI-db.

These examples shows that some modules depend on the current URI behavior, so I think URI should keep the current behavior.

nfg commented 6 years ago

That's a good point. I'll file a ticket against URI::tel, because that's where I ran into this in the first place. Cheers!