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

remove metrics code from welcome package #244

Merged
merged 1 commit into from
Dec 20, 2022

Conversation

sertonix
Copy link
Contributor

This removes some leftover code in the welcome packages that was used for metrics.

@Spiker985
Copy link
Member

And we're absolutely sure that the code that was removed was in regards to the analytics and metrics, right? It wasn't being used by some other weird event system?

@sertonix
Copy link
Contributor Author

And we're absolutely sure that the code that was removed was in regards to the analytics and metrics, right? It wasn't being used by some other weird event system?

It was only used in the metrics. You can check the consumeReporter function was called from the metrics system.

Copy link
Member

@Spiker985 Spiker985 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Places where it intersects with existing code should maintain functionality

Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

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

I'll go ahead and approve this PR then we can merge.

I do want to note that the failure rate of tests within the welcome package are higher than we would expect. But to take that with a grain of salt because these tests are highly unstable. For example the baseline taken some time ago specified an expected failure rate of 178 tests (for the welcome package group) meanwhile the PR prior to this has 480 something failing, while this has 200 something failing.

So not a blocker just pointing out. Otherwise lets merge this one

@confused-Techie confused-Techie merged commit 947a2f3 into pulsar-edit:master Dec 20, 2022
@sertonix sertonix deleted the remove-welcome-metrics branch December 20, 2022 05:56
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.

3 participants