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.

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

[Breaking] Remove the statsd metrics, dependency no more available #2423

Merged
merged 1 commit into from
Dec 19, 2022
Merged

[Breaking] Remove the statsd metrics, dependency no more available #2423

merged 1 commit into from
Dec 19, 2022

Conversation

sbrunner
Copy link
Member

@sbrunner sbrunner commented May 12, 2022 •
edited
Loading

@sbrunner sbrunner requested review from danduk82 and a user May 12, 2022 14:37
@sbrunner sbrunner force-pushed the fix-build branch 4 times, most recently from fe3a8b6 to 277e662 Compare May 12, 2022 15:32
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sbrunner sbrunner marked this pull request as draft May 12, 2022 15:46
@sbrunner sbrunner changed the title Fix the build [WIP] Change the metrics system May 13, 2022
@sbrunner sbrunner force-pushed the fix-build branch 3 times, most recently from 32eb6eb to c9d49bd Compare November 1, 2022 08:32
@sbrunner sbrunner changed the title [WIP] Change the metrics system [Breaking] Remove the statsd metrics, dependency no more available Nov 1, 2022
@sbrunner sbrunner requested a review from pmauduit November 1, 2022 10:05
@sbrunner sbrunner marked this pull request as ready for review November 1, 2022 10:06
@sbrunner sbrunner added backport 3.27 backport 3.28 Backport the pull request to the '3.28' branch backport 3.29 Backport the pull request to the '3.29' branch labels Nov 1, 2022
pmauduit
pmauduit requested changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@pmauduit pmauduit left a comment

Choose a reason for hiding this comment

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

Why not removing the StatsDReporterInit class totally ? The class looks pretty useless to me after your PR

@sbrunner
Copy link
Member Author

sbrunner commented Nov 2, 2022

Good catch @pmauduit thanks, fixed :-)

@sbrunner sbrunner removed backport 3.27 backport 3.28 Backport the pull request to the '3.28' branch backport 3.29 Backport the pull request to the '3.29' branch labels Nov 3, 2022
@sbrunner
Copy link
Member Author

@pmauduit fixed

@pmauduit
Copy link
Contributor

Looks almost good to me, but the class is still referenced here and you should end up with a ClassNotFoundException / NoClassDefFoundError if you don't remove the following line:

mapfish-print/core/src/main/resources/mapfish-spring-application-context.xml

Line 69 in befa4bb

<bean id="statsDReporterInit" class="org.mapfish.print.metrics.StatsDReporterInit" lazy-init="false"/>

@sbrunner
Copy link
Member Author

@pmauduit this is already removed, you point an old commit...

@sbrunner
Copy link
Member Author

@pmauduit up...

@sbrunner
Copy link
Member Author

sbrunner commented Dec 8, 2022

@pmauduit up...

pmauduit
pmauduit approved these changes Dec 8, 2022
View reviewed changes
Copy link
Contributor

@pmauduit pmauduit left a comment

Choose a reason for hiding this comment

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

LGTM

@sbrunner sbrunner enabled auto-merge December 9, 2022 08:13
@sbrunner sbrunner merged commit 73d2610 into master Dec 19, 2022
@sbrunner sbrunner deleted the fix-build branch December 19, 2022 08:46
@sbrunner sbrunner mentioned this pull request Aug 8, 2024
Closed
@sbrunner sbrunner mentioned this pull request Aug 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmauduit pmauduit pmauduit approved these changes

@danduk82 danduk82 Awaiting requested review from danduk82

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sbrunner @pmauduit