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

Allow to install Fastest in global mode #119

Merged
merged 1 commit into from
Dec 11, 2017
Merged

Allow to install Fastest in global mode #119

merged 1 commit into from
Dec 11, 2017

Conversation

francoispluchino
Copy link
Collaborator

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets ~
License MIT
Doc PR ~

The Symfony's team strongly recommend in Symfony 4 Flex should not add in the Phpunit dependencies in our projects, even in dev. So, it is recommended to install Phpunit globally. However, Fastest is not able to work in global mode.

This PR allow to install/use the Fastest in global mode with Phpunit.

@DonCallisto
Copy link
Collaborator

That's a good improvement but what if someone has installed phpunit globally not through composer?

Something like

$ wget https://phar.phpunit.de/phpunit-x.y.phar
$ chmod +x phpunit-x.y.phar
$ sudo mv phpunit-x.y.phar /usr/local/bin/phpunit

@francoispluchino
Copy link
Collaborator Author

Maybe in this case, use or add an environment variable to define the binary to use?

@francoispluchino
Copy link
Collaborator Author

Sorry, use directly the phpunit bin.

@DonCallisto
Copy link
Collaborator

It make sense. Could you please provide a working solution here?

@francoispluchino
Copy link
Collaborator Author

Yes, of course.

@francoispluchino
Copy link
Collaborator Author

It's done.

return ('\\' === DIRECTORY_SEPARATOR)
? 'bin\phpunit {}'
: 'bin/phpunit {}';
if (null === self::$cacheBinCmd) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you adopt an "early exit" here to compact the code?
Something like

if (null !== self::$cacheBinCmd) {
  return self::$cacheBinCmd
}

if (null === self::$cacheBinCmd) {
self::$cacheBinCmd = 'phpunit {}';

if ('\\' === DIRECTORY_SEPARATOR) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you extract this and the else in more semantic methods? Something like isRunningOnWindows, setWindowsBinCmd and isRunningOnUnix, setUnixBinCmd or whatever you like but more communicative.

Thank you.

@francoispluchino
Copy link
Collaborator Author

The requested changes are made.

@DonCallisto
Copy link
Collaborator

Last one thing, could you document this behavior?

@francoispluchino
Copy link
Collaborator Author

It's done. I don't really know what to add!

@DonCallisto DonCallisto merged commit 964e94c into liuggio:master Dec 11, 2017
@DonCallisto
Copy link
Collaborator

LGTM, thanks a lot again

@francoispluchino
Copy link
Collaborator Author

Thank you for your speed. When do you expect to create a stable release like 1.5.2?

@DonCallisto
Copy link
Collaborator

Not 1.6? However as soon as I can integrate #116 (I'll ask for your review)

@francoispluchino
Copy link
Collaborator Author

Personally, I will upgrade to version 1.6.0 because there is a new feature.

In this case, update the composer branch alias in composer.json file.

@DonCallisto
Copy link
Collaborator

I've already done in #116 last win commit, take a look

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.

2 participants