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

feat: improve IPerfumeData types and usage #267

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

Conversation

peterpeterparker
Copy link

@peterpeterparker peterpeterparker commented Aug 16, 2024

Motivation

I noticed that the IPerfumeData type was incomplete. There was some usage of any, and the type did not declare all possible properties.

This PR aims to improve this to ease the usage of the library.

Changes

  • Created the type IPerfumeStorageEstimate.
  • Extended IPerfumeData with IPerfumeStorageEstimate, IPerfumeDataConsumption, and IPerformanceEntry.
  • Replaced any usage with specific types.
  • Rewrote the function that maps the numbers in logData to comply with ESLint and type usage. A functional approach that does not modify the type passed as a parameter is preferable.

Tests

One test is failing, similar to when I ran the test suite on the main branch. Therefore, I assume that the tests are currently broken, and providing the exact same results validates the refactoring.

I did not provide an additional test since the PR introduces no new functional features.

if (typeof metric[key] === 'number') {
metric[key] = roundByFour(metric[key]);
const roundNumericProperties = (data: IPerfumeData): IPerfumeData => {
if (typeof data === 'number') {
Copy link
Author

@peterpeterparker peterpeterparker Aug 17, 2024

Choose a reason for hiding this comment

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

Note that this if statement is designed to replicate the original behavior, if I get it right. It's unclear why IPerfumeData values of type number are not rounded, unlike objects.

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.

1 participant