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

improve the Application::bootstrapWith hook to find Laravel's application name #3036

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

joke2k
Copy link
Contributor

@joke2k joke2k commented Jan 13, 2025

Description

As per #3034 this fixes the scenario in which a Laravel 11+ application does not have the config/app.php file and when in this file is not specified the "name" key (using the default from vendor/laravel/framework/config/app.php).

I was trying to cover my code with test but I cannot execute the test suite, it cannot execute some build using make due to lack of permissions 🤷‍♂️ .

The new behaviour from Laravel config loader is here: https://github.com/laravel/framework/blob/11.x/src/Illuminate/Foundation/Bootstrap/LoadConfiguration.php#L189

This is the default config:
https://github.com/laravel/framework/blob/11.x/config/app.php#L19

Fixes #3034

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@joke2k joke2k requested a review from a team as a code owner January 13, 2025 21:57
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 6.97%. Comparing base (c73b7f7) to head (ec0b059).

Files with missing lines Patch % Lines
...DTrace/Integrations/Laravel/LaravelIntegration.php 0.00% 4 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (c73b7f7) and HEAD (ec0b059). Click for more details.

HEAD has 6 uploads less than BASE
Flag BASE (c73b7f7) HEAD (ec0b059)
tracer-php 11 6
appsec-extension 1 0
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #3036       +/-   ##
============================================
- Coverage     72.93%   6.97%   -65.97%     
- Complexity     2781    2784        +3     
============================================
  Files           139     112       -27     
  Lines         15166   11020     -4146     
  Branches       1022       0     -1022     
============================================
- Hits          11062     769    -10293     
- Misses         3552   10251     +6699     
+ Partials        552       0      -552     
Flag Coverage Δ
appsec-extension ?
tracer-php 6.97% <0.00%> (-67.83%) ⬇️

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

Files with missing lines Coverage Δ
...DTrace/Integrations/Laravel/LaravelIntegration.php 0.00% <0.00%> (-80.89%) ⬇️

... and 107 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c73b7f7...ec0b059. Read the comment docs.

Copy link
Contributor

@PROFeNoM PROFeNoM left a comment

Choose a reason for hiding this comment

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

It's simple and looks fine to me! Thanks 😃

@PROFeNoM PROFeNoM merged commit 12e3d77 into DataDog:master Jan 16, 2025
597 of 648 checks passed
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.

[Bug]: Laravel 11 breaks the integration
3 participants