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

[CCI] Update faker dependency #653

Closed
wants to merge 1 commit into from
Closed

[CCI] Update faker dependency #653

wants to merge 1 commit into from

Conversation

andreymyssak
Copy link
Collaborator

Description

Update faker dependency to 5.5.3

Issues Resolved

#594

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • All tests pass
    • yarn lint
    • yarn test-unit
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@andreymyssak @SergeyMyssak
Update faker dependency (opensearch-project#594)
eceead2 
Co-authored-by: Sergey Myssak <[email protected]>
Signed-off-by: Andrey Myssak <[email protected]>
@andreymyssak andreymyssak requested a review from a team as a code owner March 31, 2023 14:21
@andreymyssak andreymyssak mentioned this pull request Mar 31, 2023
Closed
6 tasks
@BSFishy
Copy link
Contributor

BSFishy commented Mar 31, 2023

According to #594 (comment), faker is no longer supported. Why update that dependency instead of migrating to @faker-js/faker? Is this an intermediate step because the migration will take more effort?

@andreymyssak
Copy link
Collaborator Author

andreymyssak commented Apr 1, 2023 •
edited
Loading

According to #594 (comment), faker is no longer supported. Why update that dependency instead of migrating to @faker-js/faker? Is this an intermediate step because the migration will take more effort?

This was done intentionally, since migrating can take longer and make a lot of changes, I had in mind to do it in the new PR. What do you think, maybe we should migrate this right away?

UPD: The migration turned out to be not difficult #655. In case we go the migration way, this PR can be closed

@KrooshalUX
Copy link
Contributor

Hey y'all - I would like to know if there are impacts to any component functions or interactions with this or any other migration.

@andreymyssak
Copy link
Collaborator Author

Hey y'all - I would like to know if there are impacts to any component functions or interactions with this or any other migration.

No, it will not impact the components and their interaction, the specification of the components remains the same.

@andreymyssak andreymyssak mentioned this pull request Apr 1, 2023
Merged
6 tasks
@BSFishy
Copy link
Contributor

BSFishy commented Apr 5, 2023

Hey y'all - I would like to know if there are impacts to any component functions or interactions with this or any other migration.

It looks like the only place where faker is used is to generate realistic-looking fake data for the docsite for presentation purposes. Nothing related to component functionality or interaction. Most likely closing this in favor of #655 anyway

@ashwin-pc ashwin-pc added the CCI College Contributor Initiative label Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CCI College Contributor Initiative
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andreymyssak @BSFishy @KrooshalUX @ashwin-pc