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

Laravel 7 Support #10

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

antonioribeiro
Copy link

Forked, merged some of the relevant PRs you had here and added support for Laravel 7.

Also fixed an error when the key was wrong (not empty), it would not reach the exception handler reporter, throwing the exception over and over, and just terminating the app with a silent error 500.

@kenanaReda
Copy link

@antonioribeiro Thanks for the fixing!
How could I install this package but with those commits that you've added?
I assume I should run
composer require marchie/ms-application-insights-laravel@dev-master@90edf0a but since this commit is not merged yet in master and it's from forked repository, so this won't work
Thanks!

@antonioribeiro
Copy link
Author

No problem!

You can probably do it this way:

rm -rf vendor/marchie/ms-application-insights-laravel
composer update --prefer-source
cd vendor/marchie/ms-application-insights-laravel
git fetch origin pull/10/head:pull-10
git checkout pull-10

Cheers!

@kenanaReda
Copy link

Thanks for answering! @antonioribeiro
Actually, I was also trying to use this PR that you've made
microsoft/ApplicationInsights-PHP#74
It seems it still limits the guzzle version to be between 5 and 6.2
and I need to install a higher version ^6.3 since it's a dependency for another package.
so After I've check out PR 74, I've tried to run
composer require guzzlehttp/guzzle:6.3 -W
but getting the same old issue
Problem 1
- microsoft/application-insights 0.3.3 requires guzzlehttp/guzzle >=5.0 <=6.2.0 -> found guzzlehttp/guzzle[5.0.0, ..., 5.3.4, 6.0.0, ..., 6.2.0] but it conflicts with your root composer.json require (6.3).
- marchie/ms-application-insights-laravel v0.2.4 requires microsoft/application-insights ^0.3.0 -> satisfiable by microsoft/application-insights[0.3.3].
- marchie/ms-application-insights-laravel is locked to version v0.2.4 and an update of this package was not requested.

@antonioribeiro
Copy link
Author

This is a composer.json you can use to make it install Laravel 7 and Guzzle 6.3+:

{
    "require": {
        "marchie/ms-application-insights-laravel": "dev-feature/laravel-7"
    },
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/antonioribeiro/ms-application-insights-laravel.git"
        }
    ]
}

@Eventpicker
Copy link

Is there a solution for Laravel 8 as well?

@johnwc
Copy link
Collaborator

johnwc commented Nov 21, 2023

@antonioribeiro composer.lock should not be deleted. It is fundamental part of building packages.

@antonioribeiro
Copy link
Author

Hey @johnwc, thanks for your input!

The composer.lock/no-composer.lock committed is being debated for years. The general understanding today is that for applications, being deployed by via a repository, you should commit the composer.lock as it will assure the application will always deploy with those locked versions. But for packages and libraries you should not, because, first, Composer will not really use it, it has to recalculate all dependencies for each project, and also if you are a package developer making changes to the package, having the composer.lock file can only harm you, as a composer install will probably install very old dependencies, and, if a dependency of your package breaks your package in the future, you will also never know, if the test suit is doing a composer install instead of a composer update before running tests. And if it's actually doing a composer update, the composer.lock files makes even less sense as it not being actually used anywhere during package development/testing, right.

If you look at the repositories of the largest Laravel package developers, like Spatie or even Laravel, you won't probably find any composer.lock file committed to their package repositories.

Cheers!

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.

5 participants