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

Change data storage to files #461

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

Conversation

laraujo7
Copy link
Contributor

I've added:

  • USAGE.md docs if applicable
  • CHANGELOG.md

@laraujo7
Copy link
Contributor Author

laraujo7 commented May 17, 2022

@vbrazo So, I wondered if we could move the data passed in the functions to a file, which would improve the reading of the modules.
This was my attempt, but I didn't know which type of file to use, so I left it as a txt. Do you think a yml or a json file would be better?

I would be down to change everything if it gets accepted 👍.

Copy link
Member

@vbrazo vbrazo left a comment

Choose a reason for hiding this comment

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

Could you please provide benchmarks to show if this approach is faster than the current one?

I'd also like to get an opinion from @igas or @anthonator on this PR.

@laraujo7
Copy link
Contributor Author

laraujo7 commented May 17, 2022

So, I made some changes after running the benchmark as I found out that my solution was a bit slower. After my optimization, this was the result.

Screenshot 2022-05-17 at 17 55 44

What do you think @vbrazo ?

@laraujo7 laraujo7 changed the title Data storage Change data storage to file May 17, 2022
@laraujo7 laraujo7 changed the title Change data storage to file Change data storage to files May 17, 2022
Copy link
Member

@vbrazo vbrazo left a comment

Choose a reason for hiding this comment

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

@laraujo7 thanks for adding the benchmark.

@laraujo7 laraujo7 force-pushed the data_storage branch 3 times, most recently from e604590 to 2a9a44e Compare June 11, 2023 15:55
@laraujo7 laraujo7 force-pushed the data_storage branch 2 times, most recently from d7214bb to 24d0b82 Compare June 11, 2023 16:03
@nelsonmestevao
Copy link

This would make contributing to Faker much easier! 🙏

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.

None yet

3 participants