artesaos / seotools

SEO Tools for Laravel
MIT License
3.12k stars 503 forks source link

Default Values #240

Closed iBotPeaches closed 3 years ago

iBotPeaches commented 3 years ago

What steps will reproduce the problem?

Install the plugin

What is the expected result?

Empty defaults (Or even pulled from Laravel config)

What do you get instead?

Some weird defaults that do the opposite of helping SEO.


<title>It's Over 9000!</title>
--
  | <meta name="description" content="For those who helped create the Genki Dama">
  | <meta property="og:title" content="Over 9000 Thousand!" />
  | <meta property="og:description" content="For those who helped create the Genki Dama" />

Why does the plugin do this? I shouldn't be required to publish configurations just to tweak values away from non-standard defaults.

peterangelov commented 3 years ago

Those values are exactly from the config file. Look at https://github.com/artesaos/seotools/blob/master/src/resources/config/seotools.php

iBotPeaches commented 3 years ago

Those values are exactly from the config file. Look at https://github.com/artesaos/seotools/blob/master/src/resources/config/seotools.php

I'm aware, but why? What are other products have you installed that defaults are just wrong? These aren't defaults, these are incorrect values that must change. I would argue that blank values are a better default.

peterangelov commented 3 years ago

🤷‍♂️ probably question to package author.

vinicius73 commented 3 years ago

You can just publish your config files. The default exist to show how that lib works, and you can change anything for your use case.

That not is a real "big thing", just change config values. If your change do not work, that is a bug and you can submit an issue/pr :+1:

iBotPeaches commented 3 years ago

Respectfully I disagree, but your package - your rules so I understand.

I took a stab at fixing this with a PR, but all tests depend on the default configurations so not something I can spend time on unforunately as I'm not aware of best method for tests to be loaded with a different configuration.

If I could leave you with another example. There aren't many packages in my memory, if any, that require you to publish configurations. Configurations are supposed to be a good default - only publishing and changing them (if required). Imagine if you installed Laravel Cashier, but the Stripe keys were hard-coded and you had to publish the configuration to change them to your own.

This is basically what is happening here. Sure, its easy to publish a configuration and change it, but alas. Thanks for the package. I do appreciate it, just voicing an opinion.

vinicius73 commented 3 years ago

That is not my package :wink: , it is a community package, I just keep it working. I don't know what laravel packages you know.

How i've said, the default exist to be changed. It is because every project have a unique title and description, blank values could confuse the package users.

We appreciate your opinion. If you have other ideias feel free to share with us.