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

chore: deprecate LoggerOps #32513

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

sebastian-alfers
Copy link
Contributor

Fixes #32500

Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

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

A few usages in the samples as well (akka-sample-sharding-scala)

* or
* {{{
*import akka.actor.typed.scaladsl._
* }}}
*
* @param log the underlying [[org.slf4j.Logger]]
*/
@deprecated("Not needed in 2.13 and later", "2.10.0")
Copy link
Member

Choose a reason for hiding this comment

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

Isn't deprecating the class enough (and implicitly means all methods are deprecated as well)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was not sure. If you suggest, I can use that single-deprecation approach.

Copy link
Member

Choose a reason for hiding this comment

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

Just the class is enough

@@ -17,15 +17,15 @@ package object scaladsl {
* Enable these extension methods with:
*
* {{{
* import akka.actor.typed.scaladsl.LoggerOps
* }}}
* * }}}
Copy link
Member

Choose a reason for hiding this comment

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

unintended change?

@sebastian-alfers
Copy link
Contributor Author

A few usages in the samples as well (akka-sample-sharding-scala)

I looked again, but did not find 🙈 Can you point out a sample?

@@ -57,11 +56,11 @@ private class WeatherStation(context: ActorContext[WeatherStation.Command], wsid
private val stationUrl = s"http://${settings.host}:${httpPort}/weather/$wsid"

def running: Behavior[WeatherStation.Command] = {
context.log.infoN(s"Started WeatherStation {} of total {} with weather port {}",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@johanandren Isn't this what you refer to?

Copy link
Member

Choose a reason for hiding this comment

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

Ah indeed, sorry, everything is good then 👍

Copy link
Member

@johanandren johanandren left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -57,11 +56,11 @@ private class WeatherStation(context: ActorContext[WeatherStation.Command], wsid
private val stationUrl = s"http://${settings.host}:${httpPort}/weather/$wsid"

def running: Behavior[WeatherStation.Command] = {
context.log.infoN(s"Started WeatherStation {} of total {} with weather port {}",
Copy link
Member

Choose a reason for hiding this comment

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

Ah indeed, sorry, everything is good then 👍

@johanandren johanandren merged commit a5801c0 into akka:main Sep 9, 2024
5 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.

deprecate logger ops, only needed for Scala 2.12
2 participants