zoopcommerce / shard

Add new behaviours to Doctrine Mongo ODM Documents
MIT License
4 stars 1 forks source link

Hotfix: EmbedOne Unserializer #18

Closed joshystuart closed 10 years ago

joshystuart commented 10 years ago

Fixes #17;

The following now works:

superdweebie commented 10 years ago

@crimsonronin fantastic. Patch looks great. You can see on the travis report (https://travis-ci.org/zoopcommerce/shard/jobs/14895632) that there are only a few CS fixes, and then the patch will be green and ready for merge.

joshystuart commented 10 years ago

Thanks yeah.

I was just going to run php /usr/local/bin/php-cs-fixer.phar fix /mnt/www/zoop/vendor/zoopcommerce/shard/tests --level=psr2

Is that what you do? Or does it miss some things?

superdweebie commented 10 years ago

php-cs-fixer gets most things, but not everything.

Have a look in .travis.yml in shard's root directory. If you want, you can run the commands locally to check that phpcs will pass before pushing up to github and waiting for the travis job to finish.

joshystuart commented 10 years ago

So you just manually fix those then?

superdweebie commented 10 years ago

What I would do is run php-cs-fixer, then run phpcs locally, and fix the one or two things that php-cs-fixer missed by hand.

joshystuart commented 10 years ago

Sorry for my noobiness, but I don't have phpcs locally. Should I add it to my composer? Or how should I set this all up?

Currently I'm just working from my composer vendor directory in the Zoop repo. Should it be "stand alone"?

superdweebie commented 10 years ago

No problem with the questions. :)

If you composer install shard with development dependencies, then phpcs will be loaded for you (phpcs is php_codesniffer under require-dev). Then, from the tests directory you can just run the command from the travis.yml: ../vendor/bin/phpcs --standard=PSR2 ../lib/ ./Zoop/.

joshystuart commented 10 years ago

Thanks. I'll give that a go.

superdweebie commented 10 years ago

Sorry if the automated code quality checks are annoying. It took me some time to bring the shard repo up to standard so they passed, but I think it has really been worth the effort.

joshystuart commented 10 years ago

It'd be good to try and automate some of these. Not sure if the netbeans formatter is customizable enough to do it, or if there's a plugin.

superdweebie commented 10 years ago

I haven't gone looking for a netbeans PSR02 formatter. There might be one out there.

joshystuart commented 10 years ago

composer.phar install should automatically pull the dev dependencies shouldn't it?

superdweebie commented 10 years ago

Yes. It should say installing with dev dependencies ... or something like that.

joshystuart commented 10 years ago

:S It's still not installing phpcs

Installing dependencies (including require-dev) from lock file

It's in my lock file too:

"require-dev": {
                "squizlabs/php_codesniffer": "1.4.*"
            },
superdweebie commented 10 years ago

Why not try a composer update to refresh everything, including the lock file?

joshystuart commented 10 years ago

Yeah I tried that first. I'll try again.

_First world composer problems_

superdweebie commented 10 years ago

I'll give it a go my end.

joshystuart commented 10 years ago

No dice again. Might have something to do with composer cache?

superdweebie commented 10 years ago

I just did the following and it worked for me:

git clone http://github.com/zoopcommerce/shard
cd shard
composer.phar update

A batch file for phpcs is placed in vendor/bin and the sources live in vendor/squizlabs/php_codesniffer.

joshystuart commented 10 years ago

Ah, right. So you are working with a "stand alone" version of shard. I was working within my Zoop repo (as stated above).

I'll do that now and try again.

Thanks

superdweebie commented 10 years ago

Yes, that's the way to do it. You can place your 'stand alone' shard in zoop/vendor/zoopcommerce/shard so you have the best of both worlds.

superdweebie commented 10 years ago

BTW, when composer installs dev packages, it only installs dev packags in the root json. It doesn't install dev packages of dependencies - and if you think about it, that makes sense.

joshystuart commented 10 years ago

Yeah, that's what I was getting caught out on. They installed correctly as a "stand alone". I'll fix and send updates through soon.