search

giftcards / FixedWidth

Library for working with fixed width files as well as their associated specs
MIT License
10 stars 4 forks source link

Move symfony/yaml to require from require-dev #13

Open lasselehtinen opened 8 years ago

lasselehtinen commented 8 years ago

Hi,

Seems like symfony/yaml is in require-dev when it is actually a required library even for basic usage. This comes up when I am doing a composer update with --no-dev, which is kind of standard for production deployments. I would have made a PR, but I am not sure of other libraries are actually required as well.

  [Symfony\Component\Debug\Exception\FatalThrowableError]
  Fatal error: Class 'Symfony\Component\Yaml\Parser' not found

Exception trace:
 () at /var/www/deployment/releases/20160531112827/vendor/giftcards/fixed-width/                                                                                                                                                          src/Giftcards/FixedWidth/Spec/Loader/YamlSpecLoader.php:28
 Giftcards\FixedWidth\Spec\Loader\YamlSpecLoader->loadSpecFile() at /var/www/dep                                                                                                                                                             loyment/releases/20160531112827/vendor/giftcards/fixed-width/src/Giftcards/Fixed                                                                                                                                                             Width/Spec/Loader/AbstractSpecFileLoader.php:32
 Giftcards\FixedWidth\Spec\Loader\AbstractSpecFileLoader->initializeSpec() at /v                                                                                                                                                             ar/www/deployment/releases/20160531112827/vendor/giftcards/fixed-width/src/Giftc                                                                                                                                                             ards/FixedWidth/Spec/Loader/ArraySpecLoader.php:33
 Giftcards\FixedWidth\Spec\Loader\ArraySpecLoader->loadSpec() at /var/www/deploy                                                                                                                                                             ment/releases/20160531112827/vendor/giftcards/fixed-width/src/Giftcards/FixedWid                                                                                                                                                             th/Spec/FileFactory.php:56
yjv commented 8 years ago

the reason i did it that way was because its only required for the yaml file loading. If you decide to load a spec some other way or build it in code then loading the yaml lib would be useless. Then there would be a bunch of useless deps that are only there for an optional feature.

Im open to changing that if that makes more sense though.

Thanks!

lasselehtinen commented 8 years ago

Well generally it is considered that you should put all the dependecies used by the package to require and require-dev is meant only for the stuff that would need to actually develop the package like PHPUnit, Behat etc. That is why lot of deployment tools use the --no-dev parameter. I would include symfony/yaml in the require section, even though the package user might not use it.

https://getcomposer.org/doc/04-schema.md#require-dev

yjv commented 8 years ago

Yeah I see what you mean. The only thing is the yaml file loading is an optional feature so i didnt want to require someone to load deps for an optional feature. The reason why its in require-dev was for unit testing. I figured when someone would want to use that feature theyd do a require for those specific deps in their own project which would then make them available to the lib as well and it would work the same as if i would have put them in require as opposed to require-dev. I personally dont have much of an issue if an unnecessary dep or 2 is loaded into my project but some good devs i know and know of dont like having "trash" deps in their project. This seemed to allow for both options though i guess it does cause this annoyance. I suppose the question is am i incorrect in thinking about the yaml file loading as something optional as opposed to something more integral to the lib, because, the way i saw it people could load the specs however they wanted and the libs primary purpose was working with the file using the spec however it was loaded along with the file however it was loaded with yaml loading being an optional convenience feature.