Skip to content

Reduced the number of travis builds. #149

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Apr 24, 2017
Merged

Reduced the number of travis builds. #149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2e00824
Reduced the number of travis builds.
Nyholm Apr 14, 2017
a5e90fd
Syntax fix
Nyholm Apr 14, 2017
a530f76
Update .travis.yml
Nyholm Apr 14, 2017
339b82c
Be clearer in Travis GUI
Nyholm Apr 14, 2017
5176010
updated according to feedback
Nyholm Apr 18, 2017
145e754
Add tests for sf 3.1
Nyholm Apr 18, 2017
ca2dd68
Re lved
Nyholm Apr 19, 2017
32b6f43
Use the symfony phpunit bridge
Nyholm Apr 19, 2017
de5fd63
Added missing deps
Nyholm Apr 24, 2017
d9b5985
Use strings instead of constants
Nyholm Apr 24, 2017
d52663f
Added legacy annotation
Nyholm Apr 24, 2017
55c9f81
Merge branch 'master' into Nyholm-patch-1
Nyholm Apr 24, 2017
23a64a6
merge fix
Nyholm Apr 24, 2017
01fc201
Fix
Nyholm Apr 24, 2017
e076a32
Require finder
Nyholm Apr 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,7 @@ php:
- hhvm

env:
global:
- TEST_COMMAND="composer test"
matrix:
- SYMFONY_VERSION=3.2.*
- SYMFONY_VERSION=3.1.*
- SYMFONY_VERSION=2.8.*
- TEST_COMMAND="composer test"

branches:
except:
Expand All @@ -29,10 +24,18 @@ matrix:
fast_finish: true
include:
- php: 5.5
env: COMPOSER_FLAGS="--prefer-stable --prefer-lowest" COVERAGE=true TEST_COMMAND="composer test-ci" SYMFONY_VERSION=2.8.*
env: COMPOSER_FLAGS="--prefer-stable --prefer-lowest" COVERAGE=true TEST_COMMAND="composer test-ci"
- php: 7.1
env: DEPENDENCIES=dev
# Test against LTS versions
- php: 7.0
env: SYMFONY_VERSION=2.8.*

before_install:
- if [ "$SYMFONY_VERSION" != "" ]; then composer require "symfony/symfony:${SYMFONY_VERSION}" --no-update; fi;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should change this to require only useful bundles instead of the whole symfony?

Copy link
Contributor

Choose a reason for hiding this comment

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

too hard to maintain (has to be edited each time the composer.json changes).

the normal jobs already take care of using the normal deps with only relevant packages. These jobs are about testing the LTS support.

- if [ "$DEPENDENCIES" = "dev" ]; then perl -pi -e 's/^}$/,"minimum-stability":"dev"}/' composer.json; fi;

install:
- composer require symfony/symfony:${SYMFONY_VERSION} --no-update
- travis_retry composer update ${COMPOSER_FLAGS} --prefer-dist --no-interaction

script:
Expand Down
6 changes: 2 additions & 4 deletions Discovery/ConfiguredClientsStrategy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@
use Http\Client\HttpAsyncClient;
use Http\Discovery\HttpClientDiscovery;
use Http\Discovery\Strategy\DiscoveryStrategy;
use Symfony\Component\Console\ConsoleEvents;
use Symfony\Component\EventDispatcher\Event;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\KernelEvents;

/**
* A strategy that provide clients configured with HTTPlug bundle. With help from this strategy
Expand Down Expand Up @@ -77,8 +75,8 @@ public function onEvent(Event $e)
public static function getSubscribedEvents()
{
return [
KernelEvents::REQUEST => ['onEvent', 1024],
ConsoleEvents::COMMAND => ['onEvent', 1024],
'kernel.request' => ['onEvent', 1024],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you use strings over class constants?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I want to avoid dependency on Symfony/console and Symfony/http-kernel.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

'console.command' => ['onEvent', 1024],
];
}
}
3 changes: 3 additions & 0 deletions Tests/Unit/DependencyInjection/ConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ public function testInvalidAuthentication()
$this->assertProcessedConfigurationEquals([], [$file]);
}

/**
* @group legacy
*/
public function testBackwardCompatibility()
{
$formats = array_map(function ($path) {
Expand Down
6 changes: 6 additions & 0 deletions Tests/Unit/DependencyInjection/HttplugExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ public function testClientPlugins()
$this->assertContainerBuilderHasServiceDefinitionWithArgument('httplug.client.acme', 0, $pluginReferences);
}

/**
* @group legacy
*/
public function testNoProfilingWhenToolbarIsDisabled()
{
$this->load(
Expand Down Expand Up @@ -174,6 +177,9 @@ public function testNoProfilingWhenNotInDebugMode()
$this->verifyProfilingDisabled();
}

/**
* @group legacy
*/
public function testProfilingWhenToolbarIsSpecificallyOn()
{
$this->setParameter('kernel.debug', false);
Expand Down
6 changes: 5 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@
"php-http/guzzle6-adapter": "^1.1.1",
"php-http/react-adapter": "^0.2.1",
"php-http/buzz-adapter": "^0.3",
"symfony/symfony": "^2.8 || ^3.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

according to travis-ci, we have either a hard dependency on TwigBundle or its at least optional and needed for testing. if its optional we should add it to the (not yet existing) suggest section along with an explanation what its needed for.

Copy link
Contributor

Choose a reason for hiding this comment

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

Twig is required for the profiler. I think we should depends on the WebProfilerBundle, but I think this will not be enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

The WebProfilerBundle only depends on the twig-bridge. But it really require the twig service (https://github.com/symfony/web-profiler-bundle/blob/master/Resources/config/profiler.xml#L11).
We probably could suggest WebProfilerBundle and TwigBundle both for using the profiler.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, WebProfilerBundle does not introduce a hard requirement on TwigBundle to make it usable in Silex. But using it in the fullstack context actually needs TwigBundle too (as it is the one configuring Twig). There is an issue about that on Symfony

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds fair to me to suggest both packages and adding them to require-dev. Looks better than depending on symfony/symfony.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, much better

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you. I've required symfony/twig-bundle, symfony/twig-bridge and symfony/web-profiler-bundle

"symfony/phpunit-bridge": "^3.2",
"symfony/twig-bundle": "^2.8 || ^3.0",
"symfony/twig-bridge": "^2.8 || ^3.0",
"symfony/web-profiler-bundle": "^2.8 || ^3.0",
"symfony/finder": "^2.7 || ^3.0",
"polishsymfonycommunity/symfony-mocker-container": "^1.0",
"matthiasnoback/symfony-dependency-injection-test": "^1.0"
},
Expand Down