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

Documentation Update #1869

Closed
TimotiusAnrez opened this issue Feb 22, 2023 · 11 comments · Fixed by #1918
Closed

Documentation Update #1869

TimotiusAnrez opened this issue Feb 22, 2023 · 11 comments · Fixed by #1918
Assignees
Labels
c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Milestone

Comments

@TimotiusAnrez
Copy link

So I have just use faker again today, and found out that person module has been change to name. Might want to change the docs a bit to help others save a bit of time while creating their faker app.

@ST-DDT
Copy link
Member

ST-DDT commented Feb 22, 2023

Which version of faker are you referring to?
Where did you look for the documentation?
Have you seen this page? https://next.fakerjs.dev/guide/upgrading.html#deprecations-and-other-changes
Do you have a suggestion on how to improve the documentation?

@TimotiusAnrez
Copy link
Author

I have seen the documentation, and it is already updated to the latest version. But the one in readme.md in the repo still has the old version I think

image

here is the image.

I know this is not a big deal, and if people look for the real documentation it will show the right modules to use. But for beginner or for others that just rely on the repo readme.md will be confuse.

Although it is just a minor changes, but it will give a better experience

@ST-DDT
Copy link
Member

ST-DDT commented Feb 22, 2023

I think you are mistaken here (person is the new module name).

The readme on the default branch (next) shows the content of the next version v8.0.0 that you can test as an alpha already. We explicitly added a version box at the top that allows you to choose which version you want to read.

20230222_162617

I dont know any way to make this more obvious.
Do you have a suggestion?

If you visit the npm page then it also shows the correct version: https://www.npmjs.com/package/@faker-js/faker

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation s: awaiting more info Additional information are requested p: 1-normal Nothing urgent labels Feb 22, 2023
@matthewmayer
Copy link
Contributor

I think the problem is that if someone skims the documentation, then they will

  1. npm install @faker-js/faker (which doesn't install alpha versions, so they get 7.6.0)
  2. look for the first example in the README, which refers to faker 8.0.0, so they use faker.person.* instead of faker.name.*

Because our major versions only come every few months, and Github defaults to showing the "next" branch, you have this potential period of a few months where README is ahead of the code.

The section at the top of the README is helpful, BUT when i read READMEs, I generally just skim and look at the code examples.

I think the best solution might be to either
a) dramatically shorten the README, not include any code examples, and just link to the current and next versions of the proper documentation.
or
b) add comments to the code examples in README like //requires Faker 8.0.0 alpha version, prior to Faker 8.0.0 use faker.name.fullName() etc

Bear in mind for every person who gets confused like this like the OP and raise an issue, there are probably 100 people who get confused by this and give up!

@matthewmayer
Copy link
Contributor

we will run into similar issues when #1735 lands - the code in the README for picking locales won't work with the version most people end up installing,

@ST-DDT ST-DDT moved this to Todo in Faker Roadmap Feb 22, 2023
@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label Feb 22, 2023
@TimotiusAnrez
Copy link
Author

TimotiusAnrez commented Feb 22, 2023

wow.. I didn't know that this issue become more serious that I thought 🥲

@ST-DDT
Copy link
Member

ST-DDT commented Feb 22, 2023

Maybe, if we add a little warning indicator there?

grafik

(Not sure about the warning text)

@TimotiusAnrez
Copy link
Author

that could work, from user point of view (me, to be honest). I tend to skim readme to get to the information that I need like @matthewmayer described. Having a warning that can tell me that you need to look at the docs to use it according to the version that I install definitely help.

@ST-DDT ST-DDT removed the s: awaiting more info Additional information are requested label Feb 26, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Feb 28, 2023

I created #1886 to address this issue. If you think we need to add the hint to more places, please let us know.

@ST-DDT ST-DDT self-assigned this Feb 28, 2023
@ST-DDT ST-DDT moved this from Todo to Awaiting Review in Faker Roadmap Feb 28, 2023
@TimotiusAnrez
Copy link
Author

sure thing

@ST-DDT ST-DDT removed their assignment Mar 9, 2023
@xDivisionByZerox
Copy link
Member

Team decision

We should provide a PR that shortens the README by removing all unnecessary code examples.

@xDivisionByZerox xDivisionByZerox added s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision labels Mar 9, 2023
@github-project-automation github-project-automation bot moved this from Awaiting Review to Done in Faker Roadmap Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants