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

feat(packages/sui-react-web-vitals): rollback to log targets with empty values again #1736

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(packages/sui-react-web-vitals): rollback to log targets with empty values again #1736

wants to merge 1 commit into from

Conversation

nucliweb
Copy link
Member

@nucliweb nucliweb commented Mar 7, 2024 •
edited

Description

The empty values are a bug of the library, in principle, we eliminate them for not sending them to Open Search, and eliminate noise, but not having the target empties gives us a distortion of the real percentages of the attributions, and that is why we resubmit them.

jordevo
jordevo approved these changes Mar 8, 2024
View reviewed changes
Copy link
Contributor

@jordevo jordevo left a comment •
edited

Choose a reason for hiding this comment

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

how's that? TBH I didn't really get why wouldn't we log those empty values...

@nucliweb
Copy link
Member Author

nucliweb commented Mar 8, 2024 •
edited

how's that? TBH I didn't really get why wouldn't we log those empty values...

The empty values are a bug of the library, in principle, we eliminate them for not sending them to Open Search, and eliminate noise, but not having the target empties gives us a distortion of the real percentages of the attributions, and that is why we resubmit them.

I just added the explanation as a description in the the PR 😜

@jordevo
Copy link
Contributor

jordevo commented Mar 8, 2024

how's that? TBH I didn't really get why wouldn't we log those empty values...

The empty values are a bug of the library, in principle, we eliminate them for not sending them to Open Search, and eliminate noise, but not having the target empties gives us a distortion of the real percentages of the attributions, and that is why we resubmit them.

I just added the explanation as a description in the the PR 😜

that's what I thought in the first place... if we were to remove empty values, there'd be an overrepresentation of the elements that get their attribution and name correctly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jordevo jordevo jordevo approved these changes

@sui-bot sui-bot Awaiting requested review from sui-bot sui-bot is a code owner

@kikoruiz kikoruiz Awaiting requested review from kikoruiz kikoruiz is a code owner

@andresz1 andresz1 Awaiting requested review from andresz1 andresz1 is a code owner

@carlosvillu carlosvillu Awaiting requested review from carlosvillu carlosvillu is a code owner

At least 2 approving reviews are required to merge this pull request.

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@nucliweb @jordevo