Skip to content

Conversation

zsistla
Copy link
Contributor

@zsistla zsistla commented Dec 19, 2024

Concerns about future/past proofing to the checking prioritized the non-eval implementation vs using the eval method for checking the version. However, aws-sdk-php internal functions frequently change without notice in the release notes. An extra eval is used to avoid the fatal error that would occur in the VERY unlikely event of Aws/Sdk.php not being found and which addresses the previous concerns while decoupling versioning from internal functions.

@newrelic-php-agent-bot
Copy link

newrelic-php-agent-bot commented Dec 19, 2024

Test Suite Status Result
Multiverse 7/7 passing
SOAK 62/62 passing

@zsistla zsistla added this to the next-release milestone Dec 19, 2024
@zsistla zsistla requested review from mfulb, lavarou and hahuja2 December 19, 2024 23:19
@codecov-commenter
Copy link

codecov-commenter commented Dec 20, 2024

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.61%. Comparing base (aae7611) to head (4804f93).

Files with missing lines Patch % Lines
agent/lib_aws_sdk_php.c 71.42% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #1001   +/-   ##
=======================================
  Coverage   78.60%   78.61%           
=======================================
  Files         196      196           
  Lines       27076    27069    -7     
=======================================
- Hits        21283    21280    -3     
+ Misses       5793     5789    -4     
Flag Coverage Δ
agent-for-php-7.2 78.62% <71.42%> (+<0.01%) ⬆️
agent-for-php-7.3 78.64% <71.42%> (+<0.01%) ⬆️
agent-for-php-7.4 78.34% <71.42%> (+<0.01%) ⬆️
agent-for-php-8.0 78.36% <71.42%> (+<0.01%) ⬆️
agent-for-php-8.1 78.35% <71.42%> (+<0.01%) ⬆️
agent-for-php-8.2 77.94% <71.42%> (+<0.01%) ⬆️
agent-for-php-8.3 77.94% <71.42%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@lavarou lavarou left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this!

Concerns about future/past proofing to the checking prioritized the non-eval implementation vs using the eval method for checking the version.
However, aws-sdk-php internal functions frequently change without notice in the release notes.
An extra eval is used to avoid the fatal error that would occur in the VERY unlikely event of Aws/Sdk.php not being found and which addresses the previous concerns while decoupling versioning from internal functions.
@zsistla zsistla merged commit d6db53b into dev Dec 20, 2024
55 of 56 checks passed
@zsistla zsistla deleted the aws_sdk_versioning branch December 20, 2024 19:41
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.

5 participants