Some (hopefully constructive) criticism of this package

[uuf6429]

1. There is an expirable() blueprint method, but no opposite (removeExpirable() or similar):

        public function removeExpirable()
        {
            return function ($columnName = 'expires_at') {
                return $this->dropColumn($columnName);
            };
        }

2. The Expirable trait is missing docs for macro/extension/magic methods (as an example, see laravel's SoftDeletes trait):

        /**
         * @mixin ExpirableEloquentQueryBuilder   <- this works because of completely overwriting query builder
         */

3. The trait has two things that makes it incompatible / risky with other package:
   1. it defines 'boot' method which means models and other traits can't use it, instead it should be bootExpirable (which is a laravel feature) - this is also how SoftDeletes trait does it
   2. Similarly, overwriting the query builder is a bad idea - it will clash with any other attempt by the end users or other 3rd-party traits. Instead, you could do it like SoftDeletingScope by adding macro methods:

      class ExpirationScope implements Scope
      {
          public function apply(Builder $builder, Model $model)
          {
              ...
          }
      
          public function extend(Builder $builder)
          {
              $builder->macro('onlyExpired', function (Builder $builder) {
                  return $builder->withoutGlobalScope(ExpirationScope::class)
                      ->where($this->model->getExpirationAttribute(), '<=', Carbon::now());
              });
              ...

      In this case, use /** @method static static|EloquentBuilder|QueryBuilder onlyExpired() */ for point 2
   3. There should be a lot of caution when adding methods to the trait - each method is yet another compatibility risk*
4. ..speaking of which, maybe check grammar - $subscriptions->expiredOnly() is more idiomatic/correct, for example
5. There's some unnecessary/redundant PHPDoc (eg, all PHPDoc in PurgeCommand and other internal classes)
6. This package clearly depends on some laravel packages (basically all the parent classes), however these dependencies are not required in composer.json

(*) this shows the problem with traits - methods in the same level cannot be overridden and in case of conflicts, you're forced to choose an implementation and you can't later on call the other methods, unlike class inheritance where you can call the parent method

The first 3 points are the most 'painful' and point 4 would be a BC break.
If I get some time later, I might try opening a PR..

---

On a different point, there's another competing package that solves some of the issues above, but has its own problems: mvdnbrk/laravel-model-expires - maybe you guys could join efforts into one solution that works well with the best of both?

[alajusticia]

Hello @uuf6429,

Thanks for your feedback, many interesting points here.

I will try to work on some of them when I get time, or you could open a PR that I will be happy to look at.

[alajusticia]

Issues resolved in v1.5

[alajusticia]

Except for point 4: the names of the methods withExpired() and onlyExpired() were chosen to follow the pattern of the SoftDeletes trait (withTrashed and onlyTrashed).
Besides it would introduce a breaking change.