search

morilog / eluquent-sluggable-persian

Laravel Eloquent sluggable for persian & arabic labguages
51 stars 16 forks source link

Some considerations #2

Open mtvs opened 9 years ago

mtvs commented 9 years ago

First thanks for contribution but I think you should reconsider these small issues.

  1. Reference the package config in the main app config dir, i.e.: app/config/packages/cviebrock/eloquent-sluggable/config.php. that user should have generated this by following the main package instructions.
  2. Instead of instructing the user to replace the whole file, it should be said that just set the method option by the provided function. This will prevent bugs like the issue#1 and other bugs that can occur if the main package config changes.

Also the default value for the second argument ($separator) is redundant, because it always receives a value from the caller. $slug = call_user_func($method, $source, $separator);

And at the end I think it fits better as a gist.

pishguy commented 6 years ago

@mtvs @morilog this configuration not work correctly on php v 7.1