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

replace athletic with phpbench #36

Merged
merged 2 commits into from
Sep 19, 2017
Merged

Conversation

bendavies
Copy link
Contributor

🚅

@bendavies bendavies force-pushed the php-bench branch 4 times, most recently from 28c4656 to b93b9c6 Compare September 19, 2017 09:41
.travis.yml Outdated
@@ -7,11 +7,12 @@ php:
before_script:
- composer self-update
- composer update
- wget https://phpbench.github.io/phpbench/phpbench.phar https://phpbench.github.io/phpbench/phpbench.phar.pubkey
Copy link
Member

Choose a reason for hiding this comment

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

Please use a composer require step in a separate build pipeline entry

Copy link
Contributor Author

@bendavies bendavies Sep 19, 2017

Choose a reason for hiding this comment

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

copied doctrine/doctrine2 here, FYI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you prefer to require in the travis step, or in composer.json?

phpbench.json Outdated
@@ -0,0 +1,13 @@
{
"bootstrap": "vendor/autoload.php",
Copy link
Member

Choose a reason for hiding this comment

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

4 spaces :-P

phpbench.json Outdated
"storage": "dbal",
"storage.dbal.connection": {
"driver": "pdo_sqlite",
"path": "tests/DoctrineTest/InstantiatorPerformance/history.db"
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't store the results unless this is committed to an alternate branch too

@bendavies bendavies force-pushed the php-bench branch 7 times, most recently from b0117e2 to b30aed2 Compare September 19, 2017 10:05
@bendavies
Copy link
Contributor Author

@Ocramius should be ok?

composer.json Outdated
"ext-pdo": "*",
"ext-phar": "*",
"phpbench/phpbench": "^0.13.0",
Copy link
Member

Choose a reason for hiding this comment

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

@bendavies to avoid not seeing different version ranges when installing phpbench or php_codesniffer, I suggest adding an explicit composer require CLI statement in the respective pipeline entries

Copy link
Contributor Author

@bendavies bendavies Sep 19, 2017

Choose a reason for hiding this comment

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

@Ocramius do you mean to require a specific version of the deps? e.g. exactly 0.13.0

Copy link
Member

Choose a reason for hiding this comment

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

No, I mean adding a composer require line to the install step of the .travis.yml build pipeline entries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. i'm not sure i know what you mean by to avoid not seeing different version ranges though...

Copy link
Member

Choose a reason for hiding this comment

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

@bendavies the point is that this lib is to be tested with all possible dependency ranges, so having a lot of dev dependencies means restricting those tested ranges. That's why I asked to move it from require-dev to a CLI command ;-)

@Ocramius Ocramius self-assigned this Sep 19, 2017
@Ocramius Ocramius added this to the 1.1.1 milestone Sep 19, 2017
@Ocramius Ocramius modified the milestones: 1.1.1, 1.2.0 Sep 19, 2017
@Ocramius
Copy link
Member

👍 🚢

Thanks @bendavies!

@Ocramius Ocramius merged commit fe8b2f5 into doctrine:master Sep 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants