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

Deprecate or replace internet.avatar in favor of image.avatar #2599

Closed
ST-DDT opened this issue Jan 9, 2024 · 1 comment · Fixed by #2601
Closed

Deprecate or replace internet.avatar in favor of image.avatar #2599

ST-DDT opened this issue Jan 9, 2024 · 1 comment · Fixed by #2601
Assignees
Labels
deprecation A deprecation was made in the PR m: internet Something is referring to the internet module p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision
Milestone

Comments

@ST-DDT
Copy link
Member

ST-DDT commented Jan 9, 2024

We already have faker.image.avatar, so IMO there is no need to have a separate implementation in the internet module.
So we should either deprecate the faker.internet.avatar method for removal, or make it an alias of faker.image.avatar.
It definitely should jsdoc link to the other method.

faker.internet.avatar:

/**
* Returns a random avatar url.
*
* @example
* faker.internet.avatar()
* // 'https://cloudflare-ipfs.com/ipfs/Qmd3W5DuhgHirLHGVixi6V76LhCkZUz6pnFt5AJBiyvHye/avatar/315.jpg'
*
* @since 2.0.1
*/
avatar(): string {
return `https://cloudflare-ipfs.com/ipfs/Qmd3W5DuhgHirLHGVixi6V76LhCkZUz6pnFt5AJBiyvHye/avatar/${this.faker.number.int(
1249
)}.jpg`;
}

faker.image.avatar(Legacy):

/**
* Generates a random avatar from `https://cloudflare-ipfs.com/ipfs/Qmd3W5DuhgHirLHGVixi6V76LhCkZUz6pnFt5AJBiyvHye/avatar`.
*
* @example
* faker.image.avatarLegacy()
* // 'https://cloudflare-ipfs.com/ipfs/Qmd3W5DuhgHirLHGVixi6V76LhCkZUz6pnFt5AJBiyvHye/avatar/170.jpg'
*
* @since 8.0.0
*/
// This implementation will change in the future when we tackle https://github.com/faker-js/faker/issues/465.
avatarLegacy(): string {
return `https://cloudflare-ipfs.com/ipfs/Qmd3W5DuhgHirLHGVixi6V76LhCkZUz6pnFt5AJBiyvHye/avatar/${this.faker.number.int(
1249
)}.jpg`;
}

Found while investigating: #2598

@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision m: internet Something is referring to the internet module deprecation A deprecation was made in the PR labels Jan 9, 2024
@ST-DDT ST-DDT added this to the v8.x milestone Jan 9, 2024
@ST-DDT ST-DDT changed the title Deprecate or replace internet.avatar in favor or image.avatar Deprecate or replace internet.avatar in favor of image.avatar Jan 9, 2024
@ST-DDT
Copy link
Member Author

ST-DDT commented Jan 9, 2024

IMO we should:

  • replace the internet.avatar implementation with image.avatar
  • let internet.avatar link to image.avatar
  • optionally deprecate internet.avatar for removal (needs decision)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation A deprecation was made in the PR m: internet Something is referring to the internet module p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant