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

PHP7 Compatibility: Rename Bool to BoolQuery and BoolFilter #837

Merged
merged 25 commits into from
May 22, 2015

Conversation

ruflin
Copy link
Owner

@ruflin ruflin commented May 11, 2015

No description provided.

…a\Filter\Bool to \Elastica\Filter\BoolFilter
@stof
Copy link
Contributor

stof commented May 11, 2015

There is one missing part: the BC layer with the old class names

@ruflin
Copy link
Owner Author

ruflin commented May 11, 2015

Jep, working on this one. I want to first check if it builds with renamed classes as my PHP 7 setup is not ready yet.

@stof
Copy link
Contributor

stof commented May 11, 2015

@ruflin you should change the Travis config to trigger builds only for the master branch and PRs targetting the master branch. This would avoid having 2 builds being triggered for your own PRs, once for the PR and once for the branch push.
Given that Travis limits the number of concurrent jobs per repo owner (i.e. among all your repos) and that each build requires 12 jobs currently (ES_COMPOSER_NODEV being true or false for 6 different PHP versions), this would make the feedback faster on Travis (you currently have 16 builds in the Travis queue, with only one of them being running because of the restriction)

@stof
Copy link
Contributor

stof commented May 11, 2015

btw, you might want to cancel builds for PRs which have been updated after that, keeping only the latest build for each PR to make the queue smaller

@ruflin
Copy link
Owner Author

ruflin commented May 11, 2015

@sten Jep. I already canceled most of the builds. It worked in the past as there were less jobs and it seems like travis gets more and more busy for open source projects: https://www.traviscistatus.com/ I'm also working on making the builds less heavy, especially the setup.

@stof
Copy link
Contributor

stof commented May 11, 2015

@ruflin is it really needed to run both values of ES_COMPOSER_NODEV for each PHP version ?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.19%) to 85.43% when pulling fe3c0d2 on php7-compatibility into 1d63a71 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 85.22% when pulling fe3c0d2 on php7-compatibility into 1d63a71 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 85.22% when pulling fe3c0d2 on php7-compatibility into 1d63a71 on master.

@ruflin
Copy link
Owner Author

ruflin commented May 13, 2015

@stof There were some differences in the past where this was needed (and we had less PHP versions). But I will also look into this during checking for build process improvements.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.47%) to 84.77% when pulling 19bb434 on php7-compatibility into 1d63a71 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) to 85.24% when pulling 19bb434 on php7-compatibility into 1d63a71 on master.

@ruflin
Copy link
Owner Author

ruflin commented May 13, 2015

Seems like there are some issue with phpunit and PHP 7. Not sure what causes the segmentation fault: https://travis-ci.org/ruflin/Elastica/builds/62187485 To debug it I must setup my local PHP 7 version.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 79541e6 on php7-compatibility into * on master*.

@ruflin ruflin changed the title Rename Bool to BoolQuery and BoolFilter PHP7 Compatibility: Rename Bool to BoolQuery and BoolFilter May 14, 2015
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling a8566e1 on php7-compatibility into * on master*.

1 similar comment
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling a8566e1 on php7-compatibility into * on master*.

@stof
Copy link
Contributor

stof commented May 18, 2015

@ruflin I suggest changing the coveralls config to report the status through the github PR status instead. Getting email notifications each time they build it is annoying (especially when they tell us they don't know what they should say)

@ruflin
Copy link
Owner Author

ruflin commented May 18, 2015

@stof I disabled comments for the moment. What I like about the comments is that it shows the coverage and the coverage changes. I'm testing codecov at the moment as there seem to be some issues with coveralls. #850

ruflin added a commit that referenced this pull request May 22, 2015
PHP7 Compatibility: Rename Bool to BoolQuery and BoolFilter
@ruflin ruflin merged commit 932015e into master May 22, 2015
@ruflin ruflin deleted the php7-compatibility branch May 22, 2015 15:17
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