Skip Menu |
 

This queue is for tickets about the Module-Implementation CPAN distribution.

Report information
The Basics
Id: 82319
Status: resolved
Priority: 0/
Queue: Module-Implementation

People
Owner: Nobody in particular
Requestors: ribasushi [...] leporine.io
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: (no value)
Fixed in: (no value)



Subject: Minimize dependency footprint of Module::Implementation
Download (untitled) / with headers
text/plain 593b
A question was raised at [1] wrt suitability of Module::Implementation as a dependency for a "low level" module. It got me curious and the result is a set of non-invasive patches leaving M::I with only one non-core dependency - Module::Runtime, which in turn has a commitment of zero-depencencies. Please let me know if the 3 patches here [2] work for you. If you have concerns about some of the changes - please let me know so I can rectify those. Cheers [1] https://rt.cpan.org/Ticket/Display.html?id=78272#txn-1160095 [2] https://github.com/ribasushi/module_implementation/commits/master
Subject: Re: [rt.cpan.org #82319] Minimize dependency footprint of Module::Implementation
Date: Sun, 30 Dec 2012 10:35:28 -0600 (CST)
To: Peter Rabbitson via RT <bug-Module-Implementation [...] rt.cpan.org>
From: Dave Rolsky <autarch [...] urth.org>
Download (untitled) / with headers
text/plain 937b
On Sat, 29 Dec 2012, Peter Rabbitson via RT wrote: Show quoted text
> A question was raised at [1] wrt suitability of Module::Implementation > as a dependency for a "low level" module. It got me curious and the > result is a set of non-invasive patches leaving M::I with only one > non-core dependency - Module::Runtime, which in turn has a commitment of > zero-depencencies. > > Please let me know if the 3 patches here [2] work for you. If you have > concerns about some of the changes - please let me know so I can rectify > those.
Haven't we gone down this road before? What's the use case for removing all these deps? I _like_ using them. It makes writing and maintaining code easier for me. -dave /*============================================================ http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) ============================================================*/
Subject: Re: [rt.cpan.org #82319] Minimize dependency footprint of Module::Implementation
Date: Mon, 31 Dec 2012 03:52:19 +1100
To: "autarch [...] urth.org via RT" <bug-Module-Implementation [...] rt.cpan.org>
From: Peter Rabbitson <ribasushi [...] cpan.org>
Download (untitled) / with headers
text/plain 728b
On Sun, Dec 30, 2012 at 11:35:44AM -0500, autarch@urth.org via RT wrote: Show quoted text
> What's the use case for removing all these deps?
So that none of these deps (nor their deps) need to think twice before relying on Module::Implementation. The very nature of M::I places it at the "bottom of the foodchain", at a similar level to Module::Runtime. Incidentally I made an identical request to Module::Runtime a year ago: https://rt.cpan.org/Public/Bug/Display.html?id=74805 I am fully aware that we have different priorities wrt the health of the near-core toolchain. Therefore all I can do is submit better code and hope you will include it. If not the case - close the ticket, and we'll rinse/repeat on some other module ;) Cheers
Download (untitled) / with headers
text/plain 577b
On Sun Dec 30 11:52:32 2012, RIBASUSHI wrote: Show quoted text
> Therefore all I can do is submit better code and > hope you will include it. If not the case - close the ticket
To clarify in case it didn't come across clearly - I would have no qualms about the patch being rejected. However, regardless which way you decide to go, it would be nice to have a definitive answer. For instance if I knew you would be opposed to the idea, I would not have made the recommendation I made in [1]. Being informed helps ;) Cheers [1] https://rt.cpan.org/Public/Bug/Display.html?id=78272#txn-1153215
Subject: Re: [rt.cpan.org #82319] [PATCH] Minimize dependency footprint of Module::Implementation
Date: Tue, 1 Jan 2013 20:47:06 -0600 (CST)
To: Peter Rabbitson via RT <bug-Module-Implementation [...] rt.cpan.org>
From: Dave Rolsky <autarch [...] urth.org>
Download (untitled) / with headers
text/plain 1.3k
On Tue, 1 Jan 2013, Peter Rabbitson via RT wrote: Show quoted text
> Queue: Module-Implementation > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=82319 > > > On Sun Dec 30 11:52:32 2012, RIBASUSHI wrote:
>> Therefore all I can do is submit better code and >> hope you will include it. If not the case - close the ticket
> > To clarify in case it didn't come across clearly - I would have no > qualms about the patch being rejected. However, regardless which way you > decide to go, it would be nice to have a definitive answer. For instance > if I knew you would be opposed to the idea, I would not have made the > recommendation I made in [1]. Being informed helps ;)
Is this change necessary for Package::Stash to use M::I? It already has non-core deps, and in fact it uses Test::Fatal, which you're trying to remove from the M::I test suite. The only new dep beyond M::I that would be added to P::S is Try::Tiny, which is already a dependency of hundreds of other modules. So I really don't see the point of removing these dependencies for the sake of P::S, whose author hasn't even asked for these changes. -dave /*============================================================ http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) ============================================================*/
Download (untitled) / with headers
text/plain 1.6k
On Tue Jan 01 21:47:18 2013, autarch@urth.org wrote: Show quoted text
> On Tue, 1 Jan 2013, Peter Rabbitson via RT wrote: >
> > Queue: Module-Implementation > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=82319 > > > > > On Sun Dec 30 11:52:32 2012, RIBASUSHI wrote:
> >> Therefore all I can do is submit better code and > >> hope you will include it. If not the case - close the ticket
> > > > To clarify in case it didn't come across clearly - I would have no > > qualms about the patch being rejected. However, regardless which way you > > decide to go, it would be nice to have a definitive answer. For instance > > if I knew you would be opposed to the idea, I would not have made the > > recommendation I made in [1]. Being informed helps ;)
> > Is this change necessary for Package::Stash to use M::I?
No, and I never claimed this. The change is necessary (imho) to make Module::Implementation more useful as a lowest level utility module. I never claimed anything more than this, hence explicitly noting that if you do not accept the patch as-is the world won't end. In fact my change to get rid of Try::Tiny had the extra benefit of attempting to expose the "should never happen" condition of an undefined $_ in a catch{}. Please consider incorporating at least some parts of _load_module_or_error() to replace the "under-the-rug sweep" added in bbaa0bac. Again - in *my* opinion, given the mission of M::I it must have an absolutely minimal dependency footprint, as long as it doesn't sacrifice its functionality and/or maintainability. My patchset does not sacrifice either, but at the end you are the primary maintainer so this decision lies with you. All I asked was a definitive yes/no, nothing more. Cheers
Download (untitled) / with headers
text/plain 122b
On Tue Jan 01 21:58:52 2013, RIBASUSHI wrote: Show quoted text
> All I asked was a definitive yes/no, nothing more.
Any word on this? :)
Subject: Re: [rt.cpan.org #82319] [PATCH] Minimize dependency footprint of Module::Implementation
Date: Fri, 4 Jan 2013 08:58:21 -0600 (CST)
To: Peter Rabbitson via RT <bug-Module-Implementation [...] rt.cpan.org>
From: Dave Rolsky <autarch [...] urth.org>
Download (untitled) / with headers
text/plain 618b
On Fri, 4 Jan 2013, Peter Rabbitson via RT wrote: Show quoted text
> Queue: Module-Implementation > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=82319 > > > On Tue Jan 01 21:58:52 2013, RIBASUSHI wrote:
>> All I asked was a definitive yes/no, nothing more.
> > Any word on this? :)
No on removing the deps, but I will look at the other changes you made. Thanks, -dave /*============================================================ http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) ============================================================*/
Download (untitled) / with headers
text/plain 653b
On Fri Jan 04 09:58:31 2013, autarch@urth.org wrote: Show quoted text
> On Fri, 4 Jan 2013, Peter Rabbitson via RT wrote: >
> > Queue: Module-Implementation > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=82319 > > > > > On Tue Jan 01 21:58:52 2013, RIBASUSHI wrote:
> >> All I asked was a definitive yes/no, nothing more.
> > > > Any word on this? :)
> > No on removing the deps,
At least consider the changes to t/taint.t - this is a test you want to run under as many wacky environments as possible, to catch early freak-fails. Test::Taint is not necessary here as you are only using it to check if -T is in effect - the kill trick is equivalent.
Download (untitled) / with headers
text/plain 171b
I incorporated some of this, but I ended up keeping Test::Taint. I realized that my taint test didn't test much, since it didn't actually use a tainted $ENV{...} value.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.