Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

adding akka diagnostics and some cleansing #274

Merged
merged 9 commits into from
Feb 2, 2023

Conversation

franciscolopezsancho
Copy link
Contributor

References akka/akka-diagnostics#19

Kept in draft until akka-diagnostics 2.0.0-M3 release

Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

looking good, feel free to cut the diagnostics M3 to unlock these sample PRs

Did you try running some of them?

akka-sample-cluster-client-grpc-scala/build.sbt Outdated Show resolved Hide resolved
@franciscolopezsancho
Copy link
Contributor Author

I did not run any of them. I was waiting to release the M3. I was wondering if only run a few or run them all. I was more inclined toward the latter.

@patriknw
Copy link
Member

patriknw commented Feb 1, 2023

Probably no need to run all, but pick a few. Those with most dependencies are more important.

val AkkaVersion = "2.7.0"
val AkkaDiagnosticsVersion = "2.0.0-M3"
val LogbackClassicVersion = "1.2.11"
val ScalaTestVersion = "3.0.8"
Copy link
Member

Choose a reason for hiding this comment

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

same thing in several places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"3.0.8" was the previous version in the build.sbt (in most build.sbt). Are you suggesting bumping it to 3.1.0 in all?

Copy link
Member

Choose a reason for hiding this comment

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

ok, while you are anyway changing it you can bump to 3.1.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is one project with '3.1.1' I'll use that for the rest.

@franciscolopezsancho franciscolopezsancho marked this pull request as ready for review February 2, 2023 09:56
@franciscolopezsancho
Copy link
Contributor Author

I checked it in quite a few projects. In a couple of them I found some unrelated issues that I raised in a separate issue.

@franciscolopezsancho
Copy link
Contributor Author

franciscolopezsancho commented Feb 2, 2023

I checked it works in quite a few projects: all good. No warning about the conf and no errors. In a couple of them I found some unrelated issues that I raised in a separate issue.

Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

LGTM

@franciscolopezsancho franciscolopezsancho merged commit a1e2c4b into main Feb 2, 2023
@franciscolopezsancho franciscolopezsancho deleted the adding_akka-diagnostics branch February 2, 2023 10:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants