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.

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

Jump to bottom

Bugfixes for PHP 5.6 #97

Merged
merged 9 commits into from
Nov 14, 2018
Merged

Bugfixes for PHP 5.6 #97

merged 9 commits into from
Nov 14, 2018

Conversation

pawelchcki
Copy link
Contributor

@pawelchcki pawelchcki commented Nov 2, 2018 •
edited
Loading

Fix 5.6 instrumentation dispatch bugs.

  • reenable tests disabled for php 5.6
  • add dockerfiles that build debug version of PHP - extremely useful for tracing

labbati
labbati previously approved these changes Nov 5, 2018
View reviewed changes
Copy link
Member

@labbati labbati left a comment

Choose a reason for hiding this comment

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

Looks good to me. Can we add it in the future, but can we think of a test for this change?

@labbati labbati mentioned this pull request Nov 5, 2018
Merged
@labbati labbati added the 🍏 core Changes to the core tracing functionality label Nov 13, 2018
@pawelchcki pawelchcki changed the title WIP Bugfixes for php5.6 Bugfixes for PHP 5.6 Nov 13, 2018
@pawelchcki pawelchcki force-pushed the bugfix/php5.6 branch 3 times, most recently from 8ab88ed to be51ab4 Compare November 13, 2018 17:20
@pawelchcki pawelchcki requested review from chuck and SammyK November 13, 2018 17:31
labbati
labbati reviewed Nov 14, 2018
View reviewed changes
Copy link
Member

@labbati labbati left a comment

Choose a reason for hiding this comment

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

Just adding a few comments to the non-C part. Better to have more expert eyes on the C-part.

tests/Integration/Integrations/MemcachedTest.php Show resolved Hide resolved
docker-compose.yml
- REDIS_HOSTNAME=redis_integration
- DDAGENT_HOSTNAME=ddagent_integration
x-aliases:
- &base_php_service
Copy link
Member

Choose a reason for hiding this comment

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

💯

docker-compose.yml Show resolved Hide resolved
@labbati labbati added this to the 0.3.0 milestone Nov 14, 2018
labbati
labbati approved these changes Nov 14, 2018
View reviewed changes
Copy link
Member

@labbati labbati left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, looks good to me on the non C-side of the things

labbati
labbati approved these changes Nov 14, 2018
View reviewed changes
Copy link
Member

@labbati labbati left a comment

Choose a reason for hiding this comment

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

Also looked at the C part and looks good to me!

@pawelchcki pawelchcki merged commit d710489 into master Nov 14, 2018
@pawelchcki pawelchcki deleted the bugfix/php5.6 branch November 14, 2018 13:45
bwoebi pushed a commit that referenced this pull request Sep 15, 2023
@cataphract
Log helper communication (#97)
609de8b 
At debug level if there was an error and at trace level o/wise.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@labbati labbati labbati approved these changes

@chuck chuck Awaiting requested review from chuck

@SammyK SammyK Awaiting requested review from SammyK

Assignees
No one assigned
Labels
🍏 core Changes to the core tracing functionality
Projects
None yet
Milestone
0.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@pawelchcki @labbati