Skip to content

additional debug logging for fsm #20954

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 15, 2016
Merged

additional debug logging for fsm #20954

merged 2 commits into from
Jul 15, 2016

Conversation

abesanderson
Copy link
Contributor

@akka-ci
Copy link

akka-ci commented Jul 13, 2016

Can one of the repo owners verify this patch?

@ktoso
Copy link
Contributor

ktoso commented Jul 13, 2016

OK TO TEST

@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins labels Jul 13, 2016
@akka-ci
Copy link

akka-ci commented Jul 13, 2016

Test PASSed.

@akka-ci akka-ci removed the validating PR is currently being validated by Jenkins label Jul 13, 2016
johanandren
johanandren reviewed Jul 14, 2016
View reviewed changes
akka-actor/src/main/scala/akka/actor/FSM.scala
@@ -784,7 +784,7 @@ trait LoggingFSM[S, D] extends FSM[S, D] { this: Actor ⇒
case a: ActorRef ⇒ a.toString
case _ ⇒ "unknown"
}
log.debug("processing " + event + " from " + srcstr)
log.debug("processing " + event + " from " + srcstr + " in state " + stateName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this to debug("processing {} from {} in state {}", event, srcstr, stateName) so that we do not construct this string even when logging is not enabled. Same below.

@johanandren
Copy link
Contributor

LGTM but please boyscout-remove the string-building when debugging not enabled, thanks!

@ktoso
Copy link
Contributor

ktoso commented Jul 14, 2016

LGTM, thanks a lot - good improvement!

@ktoso ktoso added the reviewed Ready to merge from review perspetive, but awaiting some additional action (e.g. order of merges) label Jul 14, 2016
@akka-ci akka-ci added validating PR is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed tested PR that was successfully built and tested by Jenkins validating PR is currently being validated by Jenkins labels Jul 14, 2016
@akka-ci
Copy link

akka-ci commented Jul 14, 2016

Test PASSed.

@johanandren johanandren merged commit 03fcf87 into akka:master Jul 15, 2016
@johanandren
Copy link
Contributor

Thanks @abesanderson !

@alexvaluyskiy alexvaluyskiy mentioned this pull request Mar 16, 2017
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
reviewed Ready to merge from review perspetive, but awaiting some additional action (e.g. order of merges) tested PR that was successfully built and tested by Jenkins
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@abesanderson @akka-ci @ktoso @johanandren @abesanderson-vivint