action-scheduler icon indicating copy to clipboard operation
action-scheduler copied to clipboard

Add deprecated ActionScheduler_WPCLI_Scheduler_command class

Open thenbrent opened this issue 6 years ago • 4 comments
trafficstars

Amidst some very nice refactoring, #267 removed the ActionScheduler_WPCLI_Scheduler_command class. This class was a public API and it could be instantiated or extended by 3rd party code. As a result, it needs to continue to exist to maintain backward compatibility unless we release those patches in a major release, e.g. v3.0.0. I'd prefer to ship them sooner than that, as they are good improvements.

A new file for the ActionScheduler_WPCLI_Scheduler_command class can be added in the /deprecated/ directory for this purpose. The run() method should still be callable and map to ActionScheduler_WPCLI_Command_Run:: execute() as well as calling _deprecated_function() to notify any developers that it has been deprecated and will be removed in a future major version.

/CC @JPry @crstauf .

thenbrent avatar Mar 28 '19 02:03 thenbrent

@thenbrent Just pushed https://github.com/crstauf/action-scheduler/commit/03642ee3dd34b1eb076b355da6ecfea9cfd3ec5b, which adds ActionScheduler_WPCLI_Scheduler_command into /deprecated/ directory.

The info you provided in the issue's description certainly helped, though I'm not 100% certain on some things; please review:

  1. how the file is included
  2. phpDoc blocks
  3. _deprecated_function() call is correct

I've tested by adding the following code to ActionScheduler_WPCLI:

public function run_legacy( $args, $assoc_args ) {
	$command = new ActionScheduler_WPCLI_Scheduler_command();
	$command->run( $args, $assoc_args );
}

and running wp action-scheduler run_legacy.

crstauf avatar Mar 30 '19 03:03 crstauf

Hey @crstauf, thanks for jumping on that! crstauf@03642ee looks good, but I think ActionScheduler_WPCLI_Scheduler_command will have to extend ActionScheduler_WPCLI_Command_Run to be fully backward compatible.

The main thing we're trying to avoid are fatal errors by anyone extending ActionScheduler_WPCLI_Scheduler_command or calling a method of it, and assuming that class / method will exist. That's why we need it to extend ActionScheduler_WPCLI_Command_Run - so that it continues to have all the methods it used to have.

Having _deprecated_function() in run() is good. I think _deprecated_function() should also exist in ActionScheduler_WPCLI_Scheduler_command::__construct() so that it's triggered for anything instantiating it too.

thenbrent avatar Apr 01 '19 02:04 thenbrent

Oh gosh, idk what I was thinking; will fix.

crstauf avatar Apr 01 '19 06:04 crstauf

@thenbrent https://github.com/crstauf/action-scheduler/commit/2a3112655b3cf4c9c82075e477e814c9e3f08a67 pushed.

crstauf avatar Apr 02 '19 06:04 crstauf

ActionScheduler_WPCLI_Scheduler_command is present in the current codebase, so I assume we just forgot to close this one.

barryhughes avatar Jan 25 '23 19:01 barryhughes