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

Allow psr/log v3 #1184

Merged
merged 1 commit into from Feb 3, 2022
Merged

Allow psr/log v3 #1184

merged 1 commit into from Feb 3, 2022

Conversation

ruudk
Copy link

@ruudk ruudk commented Dec 15, 2021

No description provided.

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cla-checker-service
Copy link

cla-checker-service bot commented Dec 15, 2021

💚 CLA has been signed

@ruudk
Copy link
Author

ruudk commented Dec 18, 2021

@technige What do you think? Can you approve the workflow so that we can see the tests pass?

@ruudk ruudk mentioned this pull request Dec 22, 2021
@franmomu
Copy link
Contributor

Looks like https://github.com/elastic/elasticsearch-php/blob/6d77eb5f69878fda0f7347ee2f937e1662063008/src/Elasticsearch/Common/EmptyLogger.php is implemeting LoggerInterface, so I guess a return type should be added there (maybe acceptable BC break)

@franmomu franmomu mentioned this pull request Dec 31, 2021
@technige
Copy link

@ruudk Just adding a quick acknowledgement that I've seen this, and I'll get back to you soon.

@technige
Copy link

To follow up, we have now added this to our internal TODO list for review.

cc: @ezimuel

@technige
Copy link

See also the separate changes proposed in #1190.

@franmomu
Copy link
Contributor

@ruudk I've tried to make a PR to your branch but it didn't appear on Github UI 🤔 you can copy the changes done in #1190 and I think the targeted branch should be 7.17

@ruudk ruudk changed the base branch from 7.x to 7.17 January 21, 2022 07:43
@ruudk
Copy link
Author

ruudk commented Jan 21, 2022

@technige @franmomu Rebased the PR, targeting 7.17, pulling in #1190. Please approve workflow

@vince83110
Copy link

Just because I am french and annoying, +1 for this one, I tested the PR on my side and thanks for the work, all of you!

@ezimuel ezimuel merged commit 67731d0 into elastic:7.17 Feb 3, 2022
@ezimuel
Copy link
Contributor

ezimuel commented Feb 3, 2022

Thanks @ruudk for this PR, I'm going to release it in v7.17.0.

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.

None yet

6 participants