search

browscap / browscap-php

Officially supported Browscap for PHP
http://browscap.org/
MIT License
422 stars 82 forks source link

Move Monolog to dev #660

Closed mostafasoufi closed 10 months ago

mostafasoufi commented 10 months ago

Regarding PR #465, why isn't MonoLog included in the development environment when it's used in the Commands classes for Unit tests?

Best

codecov-commenter commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (003b4d2) 36.26% compared to head (184d27d) 36.26%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## 7.5.x #660 +/- ## ========================================= Coverage 36.26% 36.26% Complexity 286 286 ========================================= Files 27 27 Lines 1183 1183 ========================================= Hits 429 429 Misses 754 754 ``` | [Flag](https://app.codecov.io/gh/browscap/browscap-php/pull/660/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=browscap) | Coverage Δ | | |---|---|---| | [phpunit](https://app.codecov.io/gh/browscap/browscap-php/pull/660/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=browscap) | `36.26% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=browscap#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mimmi20 commented 10 months ago

@mostafasoufi Monolog is used in the File src/Helper/LoggerHelper.php. This Helper is used in all Commands, not just in the Tests. If you want to make Monolog optional, you have to change that file too, and maybe all places where this Helper is used.

mostafasoufi commented 10 months ago

They are only uses in /bin/browscap-php which is not the main functionality of Browscap, isn't it?

mimmi20 commented 10 months ago

Browscap can be used in two ways:

  1. as Component for other projects, where it is installed via composer
  2. as Standalone Project

In the second Way, you can use these Commands. I do not know, if anyone uses Browscap this way.

If Monolog is transfered to the dev-section, these Commands will break. This means that will be a breaking change.

asgrim commented 10 months ago

I would suggest this is not mergable at least until the next major, since it would indeed break this functionality. It would help if we knew if people use this functionality or not 🤷

asgrim commented 10 months ago

@mostafasoufi thank you for the PR, but since this is functionality people may use, which would break, this wouldn't be acceptable until a major release really. I'd suggest making an issue to see if others have feedback about it and we can discuss what the migration path might look like. Thank you though, we really appreciate it!

mostafasoufi commented 10 months ago

I inquired because I'm planning to upgrade Browscap for WordPress plugins like WP Statistics and SlimStat Analytics. However, since the pull request (PR) is closed, it looks like I'll need to fork the project and work on my own repository.