Beefy Boxes and Bandwidth Generously Provided by pair Networks
Syntactic Confectionery Delight
 
PerlMonks  

Minor API changes in 10-year-old module

by wanna_code_perl (Pilgrim)
on Sep 05, 2019 at 14:40 UTC ( #11105668=perlquestion: print w/replies, xml ) Need Help??

wanna_code_perl has asked for the wisdom of the Perl Monks concerning the following question:

Hello Monks!

I'll save you the complicated backstory, but suffice to say I now maintain a small, unpopular CPAN module that hasn't been updated for ten years. I'm not going to name it, so I can talk freely about its shortcomings here. I also have a local version of the module I have been using in-house that pretty much got an accidental rewrite, one line at a time, over the past two years, while all of my RT tickets went unanswered. The in-house version is objectively better, (it actually has unit tests, and a ton of bugs and new features got taken care of), but the API has diverged slightly. So in order to upload a new version, I need to address the API differences.

Aside from entirely new subroutines, I haven't changed any subroutine names, arguments, or semantics, but I have changed a few error return values. Previously it was a little inconsistent with throwing exceptions vs. returning failure, so I standardized that. I also changed a few croak messages to be more descriptive, so if anyone is looking for a specific $@ string (arguably a bad idea anyway), they could have subtle bugs.

Short version of that: if someone plonked my current version of this module in place in their project as-is, it would mostly work, but the program might die due to network errors if the calling code wasn't already using eval{...}. A few edge cases might cause some errors that would be easily caught and fixed during integration testing. As far as I can tell, any potential upgrade errors would be of the "loud and obvious" variety. Still, the API has been stable for a decade, so I'd like to tread a little carefully.

How would you go about migrating an API such as this? I am in no particular rush to do anything, so doing a lengthy deprecation cycle (with migration documentation) wouldn't be a huge issue, but still a decent amount of extra work (and risk of regressions), with a real possibility that zero people will actually update this module in the next six months. Or maybe you wise monks have an even better idea, which is why I'm here!


Edit: Detailed Description of Changes

Copied from one of my replies, below:

The API changes I've made are deliberate, and ultimately for the good of the project. As an example of why I made these changes, there was a method that would:

  1. return 0; if certain arguments are invalid, croak()s if others are invalid,
  2. carp "Connection failed"; return 0; on connection failures. Yes, carp(), and
  3. Not trap upstream exceptions itself, so it would die for any number of undocumented reasons the author did not anticipate (and no unit tests either!)

Since returning non-zero had no semantic meaning beyond "nothing broke," and the distinction between return 0; v. croak() was already ill-defined, I changed this sub to always croak() on error, thinking most implementations would already be eval{...}-ing that call, or risk premature death.

That's the biggest change. The other 3-4 examples are similar but simpler (fewer cases).

The documentation for the methods was generally good, except when it came to error handling. So I'm not really even going against the existing documentation, here, though I'm not using that as an excuse to break people's code. :-)

Replies are listed 'Best First'.
Re: Minor API changes in 10-year-old module
by haukex (Chancellor) on Sep 05, 2019 at 17:41 UTC

    Are all the changes only related to die vs. return error codes? A fatal error is pretty easy to make non-fatal with eval {}, so it sounds like it wouldn't be too difficult to make your code have exactly the same API as the old module, or would it? If it's easy, then perhaps you could even consider making whether errors are fatal or not a configuration option.

    I wouldn't worry all too much about people relying on $@ values, unless specific $@ values have been documented in the module's docs.

    BTW, I think it's ok if you name the module, that might help, it's all open source anyway :-)

      The API changes I've made are deliberate, and ultimately for the good of the project. As an example of why I made these changes, there was a method that would:

      1. return 0; if certain arguments are invalid, croak()s if others are invalid,
      2. carp "Connection failed"; return 0; on connection failures. Yes, carp(), and
      3. Not trap upstream exceptions itself, so it would die for any number of undocumented reasons the author did not anticipate (and no unit tests either!)

      Since returning non-zero had no semantic meaning beyond "nothing broke," and the distinction between return 0; v. croak() was already ill-defined, I changed this sub to always croak() on error, thinking most implementations would already be eval{...}-ing that call, or risk premature death.

      That's the biggest change. The other 3-4 examples are similar but simpler (fewer cases).

      The documentation for the methods was generally good, except when it came to error handling. So I'm not really even going against the existing documentation, here, though I'm not using that as an excuse to break people's code. :-)

      BTW, I think it's ok if you name the module, that might help, it's all open source anyway :-)

      My personal choice. I'm not naming the module out of respect for the original author. I have not been able to reach them, and don't want them to hear about my feedback via this thread. I trust I've described the differences well enough for the general advice I'm asking for. (If not, please let me know!)

Re: Minor API changes in 10-year-old module
by choroba (Bishop) on Sep 05, 2019 at 19:00 UTC
    If you really want to be considerate of your users, you can release a new version that issues warnings every time your next version would behave differently.
    warn "Next version of Module::Unknown will die instead of returning un +def.\n"; return undef;

    map{substr$_->[0],$_->[1]||0,1}[\*||{},3],[[]],[ref qr-1,-,-1],[{}],[sub{}^*ARGV,3]

      That's a good way to add a deprecation warning for this sort of situation, and a simple solution. Thanks! And I have no deadline for this, so it's no skin off my asterisk to release such a version now, wait six months or more, then release another. My only hesitation is that it'd also be nice to give users a way to "opt in" to the changes ahead of time, and I was originally thinking of doing that in the code itself, but I reckoned that could get ugly, multiply the unit test requirements, and add two more likely points to introduce regressions. But you've got me thinking. Maybe append your warning with "For new code, use development version USERNAME/Module-Unknown-2.00_01.tar.gz" or something to that effect, with a blurb in the docs on how to fetch a dev build. Or is there an even cleverer way? :-)

        Well, you can. It's more work but you could make this first new version behave like the old version, with aformentioned warnings, but also support the fixed API when they use it with use Module qw(:always-die); or use Module qw(:newapi); or whatever. Then your next version can switch to new api and silently ignore that flag.


        holli

        You can lead your users to water, but alas, you cannot drown them.
Re: Minor API changes in 10-year-old module
by stonecolddevin (Parson) on Sep 05, 2019 at 15:29 UTC

    Well, one option is to release it as an entirely new module. The advantage of this is the people looking to upgrade can do so, and everyone else can putter along with their stable legacy code. You could put a reference in the original module's documentation about the new module and explain the improvements.

    Otherwise, you probably just want to make sure it's as backward compatible as possible. If it's not that popular, it's less of a risk introducing changes.

    Three thousand years of beautiful tradition, from Moses to Sandy Koufax, you're god damn right I'm living in the fucking past

      Thanks for the suggestion. Unfortunately, it's already in the ideal CPAN namespace for what it does, and from a functional standpoint, my version adds a couple of features (strict superset, new features don't affect backwards compatibility), and fixes a few bugs. I broke backward compatibility intentionally, since it was pretty much impossible to use the module's existing API without some kind of undefined behavior.

Re: Minor API changes in 10-year-old module
by tobyink (Abbot) on Sep 06, 2019 at 09:32 UTC

    Mostly summarising what other people have already said, but:

    • Yes, do it. It sounds like an improvement.

    • Document the changes clearly.

    • Bump the major version number.

    • Release a trial version (either an underscore in the version number or "-TRIAL" in the tarball name) at least a month before releasing it as stable.

    • If there are other CPAN modules that depend on your module (especially if they're popular), do the authors a favour and download those modules and test them against your trial version. Submit bug reports and maybe patches if you've broken them.

Re: Minor API changes in 10-year-old module
by jcb (Hermit) on Sep 05, 2019 at 23:17 UTC

    Other monks have given good advice, but I just want to be sure that incrementing the major version number is not forgotten in the process. Arguably, that and a nice notice in "Changes" should be all you need.

    The fact that the changes also make the API usable without relying on undefined behavior is also an argument in your favor. Generally, croaking on failure is appropriate if any reasonable application will need the func(...) or die ... pattern as is seen with Perl's own open most of the time. On the other hand, simply returning failure is better if an application might want to continue with some alternate method, as this sample from a test I recently wrote does:

    my $fh; open $fh, '<', \ $data or $fh = new IO::Uncompress::Gunzip (\ $data, Transparent => 1) or BAIL_OUT "cannot open string as file: $! $GunzipError";

    That assumes that IO::Uncompress::Gunzip has its own XS magic for reading the contents of a scalar that is independent of the PerlIO infrastructure that the Perl core uses for that feature. So even if Perl was built without PerlIO, IO::Uncompress::Gunzip can still do it. (Other code in the module under test already relies on IO::Uncompress::Gunzip to decompress input data, so it was already in my PREREQ_PM list.)

Re: Minor API changes in 10-year-old module
by Anonymous Monk on Sep 06, 2019 at 03:47 UTC

    Its simple

    Release a development version (thats with a _ 01 in version number)

    warn in the docs what the change is gonna be, fix what needs fixing

    Wait for complaints (ex 1week or 1month)

    Then in a week or month release the next stable version and you're done

    Then wait the next ten years for complaints from regular fools who dont read docs or have test suits .... ;) It really is that simple ;)

    For one example you can see about CGI changes since 4.03 2014-07-02

    4.05 (LEEJO on 2014-10-08) 4.04_05 DEV (LEEJO on 2014-10-06) 4.04_04 DEV (LEEJO on 2014-10-06) 4.04_03 DEV (LEEJO on 2014-09-29) 4.04_02 DEV (LEEJO on 2014-09-28) 4.04_01 DEV (LEEJO on 2014-09-20) 4.04 (LEEJO on 2014-09-04)

    LEEJO gets commit bit and sets about gutting ye olde tool bucketbagbox ...

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://11105668]
Front-paged by Corion
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others wandering the Monastery: (9)
As of 2019-09-17 11:30 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    The room is dark, and your next move is ...












    Results (207 votes). Check out past polls.

    Notices?