Skip to content

Conversation

tabacitu
Copy link
Member

In 3.0 we have a CRUD alias (pointing to CrudServiceProvider) that was registered for just one method - CRUD::resource(), which registers all the routes needed for a CRUD.

This PR:

  • makes CRUD::resource() stop working;
  • makes Route::crudResource() start working (doing the exact same thing);
  • eliminates the CRUD alias;

Why:

  • I don't think it makes sense to load this alias for just one method;
  • There is a more intuitive and reasonable way to achieve the same thing, since the Route class in macroable;
  • This opens up the possibility for us to use the CRUD alias for something else (not that we should, but we could);

Things we can do with the CRUD alias, since it would now be free to use:
(A) A services class (or helpers class), ex: CRUD::isCountable().
(B) A facade to the CrudPanel class (or currently running CrudPanel class). So that we can do:

// inside the ProductCrudController
CRUD::addField();
CRUD::addColumn();

// inside the views
CRUD::getEntry();

Instead of:

// inside the ProductCrudController
$this->crud->addField();
$this->crud->addColumn();

// inside the views
$crud->getEntry();

Obviously I think option B would be a better idea :-)

Thoughts? Feedback?

@tabacitu tabacitu self-assigned this Jul 13, 2019
@tabacitu tabacitu changed the title [4.0][Refactor] Rename CRUD::resource() to Route::crudResource() [4.0][Refactor][Ready] Rename CRUD::resource() to Route::crudResource() Jul 13, 2019
@tabacitu
Copy link
Member Author

Let's close this in favour of #1937 - which does this, but also more. MUCH more :-)

@tabacitu tabacitu closed this Jul 15, 2019
@tabacitu tabacitu deleted the route-macro branch July 15, 2019 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant