Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Source to Newly Merged Style Rule Additions #800

Merged
merged 1 commit into from
Sep 20, 2016

Conversation

robfrawley
Copy link
Collaborator

@robfrawley robfrawley commented Sep 19, 2016

This replaces #791 (which was done prior to the new "style rules" and "php-cs-fixer bridge" additions using a hand crafted regex of mine - I feel more comfortable having a real code tokenizer/lexer do it instead, heh).

So, this is simple a PHP CS Fixer run using our new default rule set.

While the change set is substantial in size, its actually quite simple. All the changes relate to one of the following three rules:

  • Ensure all files have the proper file-level doc block.
  • Ensure all class/method/etc doc blocks use consistent ordering of tags.
  • Ensure all imports are ordered.

The only, singular exception to my above assertion is a short-array to long-array syntax fix caught the fixer.

Hopefully people run this more often now! Remember, it's as easy as this now:

bin/php-cs-fixer fix

@robfrawley
Copy link
Collaborator Author

Build failure unrelated to this PR and fixed in #799 (see comment). I'll rebase this after #799 is merged so the tests pass.

@robfrawley robfrawley changed the title PHP-CS-Fixer run with our newly merged fixer rules! Update Source to Newly Merged Style Rule Additions Sep 19, 2016
@robfrawley robfrawley force-pushed the bugfix-phpcsfixer-run branch from dcacf9f to cec4e71 Compare September 20, 2016 12:04
@robfrawley
Copy link
Collaborator Author

@lsmith77 Re-based on master (with #799 merged) fixes tests; this should be good to go now.

@lsmith77 lsmith77 added the State: Reviewing This item is being reviewed to determine if it should be accepted. label Sep 20, 2016
@@ -132,7 +141,7 @@ public function store(BinaryInterface $binary, $path, $filter)
$this->flysystem->put(
$this->getFilePath($path, $filter),
$binary->getContent(),
['visibility' => $this->visibility]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this caught another use of short array syntax which means this isn't tested

Copy link
Collaborator Author

@robfrawley robfrawley Sep 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the the travis file always excluded flysystem from PHP 5.3 runs. I assumed FlySystem requires a higher PHP version, but if that's not the case we should obviously be testing it! I'll take a look at flysystems composer when I get a second.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lsmith77 Yup, FlySystem requires PHP > 5.4, so that's why it is excluded from 5.3 builds on travis and why the above wasn't being tested.

@lsmith77 lsmith77 merged commit 3fa96be into liip:master Sep 20, 2016
@lsmith77 lsmith77 deleted the bugfix-phpcsfixer-run branch September 20, 2016 12:47
@lsmith77 lsmith77 removed the State: Reviewing This item is being reviewed to determine if it should be accepted. label Sep 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants