phergie / phergie-irc-bot-react

IRC bot built on React
BSD 2-Clause "Simplified" License
81 stars 26 forks source link

Adding .php_cs file as proposed by enebe-nb in #20 discussion #33

Closed svpernova09 closed 8 years ago

svpernova09 commented 8 years ago

Please discuss any changes here (if we need any before this is merged)

svpernova09 commented 8 years ago

Not sure header_comment is working. I ran it a few times and it never seemed to change anything. Also unable to find any docs. Left Disabled for now

enebe-nb commented 8 years ago

I left the header_comment commented because it is a sugestion. The command php-cs-fixer fix --dry-run --diff will display the diffs without change any file, that will help to debug.

Now that i'm checking this, it only uses normal comment blocks to avoid conflict with doc blocks, but this way annotations won't work. Probably it won't help (although, the year is working).

enebe-nb commented 8 years ago

related to this: FriendsOfPHP/PHP-CS-Fixer#1297

svpernova09 commented 8 years ago

Removed header. Once we finalize the Code Standards from #20 I'll update the .php_cs as needed and commit the changes files.

svpernova09 commented 8 years ago

@enebe-nb Having some issues with this command on travis:

output=$(php php-cs-fixer.phar fix -v --dry-run); if [[ $output ]]; then while read -r line; do echo -e "$line"; done <<< "$output"; false; fi;

There's always output even if there are no changes. We need to grep for "begin diff" or something similar when there is something to fix.

I poked at it a couple times, couldn't get what would work locally to work with Travis. Possible option is not use this via travis if it's going to be a pain.

enebe-nb commented 8 years ago

this command comes from my testing with that older version, it seem the exit code from cs-fixer changed many times. Probably using only php php-cs-fixer.phar fix -v --dry-run will work fine.

I'll test it correctly.

enebe-nb commented 8 years ago

Using the command directly works, but it gets a "Undefined Class" error on hhvm

svpernova09 commented 8 years ago

Yeah I see, I'm not terribly worried about HHVM right now since we're still waiting for another issue with it to be resolved.

Looks good, thanks for the work on this!