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

Monolog 2.x compatabiliy #24

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@
"wiki": "https://github.com/koraktor/steam-condenser/wiki"
},
"require": {
"php": ">=5.4.0",
"monolog/monolog": "~1.10"
"php": "^7.2",
"monolog/monolog": "^2.0"
},
"require-dev": {
"phpdocumentor/phpdocumentor": "~2.6.1",
Copy link
Owner

Choose a reason for hiding this comment

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

This is unrelated and should go into a separate PR.

Copy link
Author

Choose a reason for hiding this comment

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

My apologies, that was removed oin my fork and I forgot to re-add that change.

Copy link
Author

Choose a reason for hiding this comment

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

So, i'm happy to make another PR for phpdoc, but what are we even using it for?

I don't see any phpdocs live anywhere, there's no docs written to the repo, what are we doing with it?

I can't install it via composer because it requires monolog 1.6, but reverting all these changes to satisfy a dev dependency just for the convenience of installing it via composer seems silly.

Thoughts?

Copy link
Owner

Choose a reason for hiding this comment

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

I think updating to 3.0.0-rc should be fine.

Copy link
Author

Choose a reason for hiding this comment

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

I actually did do that, though not via composer, let me see if that's a viable option, thanks.

Copy link
Author

Choose a reason for hiding this comment

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

I was able to achieve this, but only by setting the minimum stability to dev and telling it to prefer stable packages.

See here: https://github.com/zack6849/steam-condenser-php/tree/travis-php7

I tested it in my branch to bring all the testing code up to 7.x, I can make it a separate PR, but let's tackle that after we figure out this whole monolog thing, too many fixes in the air right now IMO

Copy link
Owner

Choose a reason for hiding this comment

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

I‘m totally fine with having these changes in a single „update dependencies“ PR.

Looks good so far.

"phpunit/phpunit": "~4.1.3"
},
"autoload": {
Expand Down
3 changes: 2 additions & 1 deletion lib/SteamCondenser.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@

namespace SteamCondenser;

use Monolog\Handler\StreamHandler;
use Monolog\Logger;

const VERSION = '1.3.9';
const VERSION = '1.3.10';

/**
* Returns a Monolog logger with the given name
Expand Down