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

Add books module #2310

Closed
ghost opened this issue Aug 13, 2023 · 26 comments · Fixed by #2949
Closed

Add books module #2310

ghost opened this issue Aug 13, 2023 · 26 comments · Fixed by #2949
Assignees
Labels
c: feature Request for new feature m: book Something is referring to the animal module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Milestone

Comments

@ghost
Copy link

ghost commented Aug 13, 2023

Clear and concise description of the problem

Add a faker.books module which allows for generating random titles, genres and publishers for books.

Suggested solution

We could add a faker.books module similar to the one provided by faker-ruby. The module could export the following methods:

  • title: returns a random book title
  • publisher: returns a random publisher
  • genre: returns a random genre
  • author: returns a random author

Alternative

I am not sure if author would be necessary since we already have faker.person.fullName.

Additional context

If the feature is accepted, I would like to work on it.

@ghost ghost added c: feature Request for new feature s: pending triage Pending Triage s: waiting for user interest Waiting for more users interested in this feature labels Aug 13, 2023
@github-actions
Copy link
Contributor

Thank you for your feature proposal.

We marked it as "waiting for user interest" for now to gather some feedback from our community:

  • If you would like to see this feature be implemented, please react to the description with an up-vote (:+1:).
  • If you have a suggestion or want to point out some special cases that need to be considered, please leave a comment, so we are aware about them.

We would also like to hear about other community members' use cases for the feature to give us a better understanding of their potential implicit or explicit requirements.

We will start the implementation based on:

  • the number of votes (:+1:) and comments
  • the relevance for the ecosystem
  • availability of alternatives and workarounds
  • and the complexity of the requested feature

We do this because:

  • There are plenty of languages/countries out there and we would like to ensure that every method can cover all or almost all of them.
  • Every feature we add to faker has "costs" associated to it:
    • initial costs: design, implementation, reviews, documentation
    • running costs: awareness of the feature itself, more complex module structure, increased bundle size, more work during refactors

View more issues which are waiting for user interest

@ST-DDT ST-DDT added p: 1-normal Nothing urgent and removed s: pending triage Pending Triage labels Aug 13, 2023
@ST-DDT ST-DDT added this to the vFuture milestone Aug 13, 2023
@xDivisionByZerox
Copy link
Member

What about ISBN which is currently located in the CommerceModule.

/**
* Returns a random [ISBN](https://en.wikipedia.org/wiki/ISBN) identifier.
*
* @param options The variant to return or an options object. Defaults to `{}`.
* @param options.variant The variant to return. Can be either `10` (10-digit format)
* or `13` (13-digit format). Defaults to `13`.
* @param options.separator The separator to use in the format. Defaults to `'-'`.
*
* @example
* faker.commerce.isbn() // '978-0-692-82459-7'
* faker.commerce.isbn(10) // '1-155-36404-X'
* faker.commerce.isbn(13) // '978-1-60808-867-6'
* faker.commerce.isbn({ separator: ' ' }) // '978 0 452 81498 1'
* faker.commerce.isbn({ variant: 10, separator: ' ' }) // '0 940319 49 7'
* faker.commerce.isbn({ variant: 13, separator: ' ' }) // '978 1 6618 9122 0'
*
* @since 8.1.0
*/
isbn(

Would that get relocated here as well?

@isaacfink
Copy link

What is the status of this request? I could use something like this now

@ST-DDT
Copy link
Member

ST-DDT commented Jan 4, 2024

What is the status of this request? I could use something like this now

This is currently waiting for user intrest so unfortunately this wont make it into the code in the next few months.
Please upvote the issue to make it easier for us to measure the community's intrest in it.

Could you please also detail which properties you need for the books?

@matthewmayer
Copy link
Contributor

It's waiting for 10 upvotes

Where faker doesn't provide the exact data you need you can generally get a good result by just using faker.helpers.arrayElement with an array of static data. Faker-Ruby has a book module so you could grab the data from there https://github.com/faker-ruby/faker/blob/main/lib/locales/en/book.yml

@ST-DDT ST-DDT modified the milestones: vFuture, v9.x May 1, 2024
@ST-DDT ST-DDT added s: accepted Accepted feature / Confirmed bug and removed s: waiting for user interest Waiting for more users interested in this feature labels May 1, 2024
@ST-DDT
Copy link
Member

ST-DDT commented May 1, 2024

Team Decision

This issue has gathered enough user interest to be considered for inclusion.
Currently, v9.0 is already big enough so we would like to delay this until v9.1+.

@cieslarmichal
Copy link
Contributor

@inkedtree are you still interested in working on this? I would like to work on this as well :)

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented May 3, 2024

Hi @cieslarmichal,
We appreciate your willingness to contribute to this issue. Please keep in mind that we are currently doing major restructures in our codebase. This may result in numerous merge conflicts with any work you undertake. Consequently, we've scheduled this issue for v9.x (subsequent to 9.0). If you remain interested despite these considerations, I would be more than happy to assign this issue to you.

@cieslarmichal
Copy link
Contributor

Ok thanks, I will wait then, I would appreciate if you ping me when I can start working on this :)

@ST-DDT
Copy link
Member

ST-DDT commented May 3, 2024

While the implementation cannot start right now, you could already make suggestions on how you want each method to behave and which methods to have exactly.

Like will book.title return a random elememt from a fixed list such as Harry Potter or will it use patterns like The {{word.noun}} of the {{animal.type}} or The adventures of {{person.firstName}} {{person.lastName}}

@ST-DDT ST-DDT assigned cieslarmichal and unassigned cieslarmichal May 3, 2024
@cieslarmichal
Copy link
Contributor

cieslarmichal commented May 4, 2024

I am actually developing book application so I know which methods could be useful, I would add methods as follows (open for more suggestions):

  • title (fixed list)
  • isbn (fixed list or generating valid isbns)
  • author (fixed list)
  • genre (fixed list)
  • publisher (fixed list)
  • translator (alias for person fullName)
  • format (paperback, hardcover or ebook)
  • pages (integer in range 50 - 1500)
  • releaseYear (integer in range 1940 - 2024)
  • series (fixed list like Harry Potter series)

@matthewmayer
Copy link
Contributor

matthewmayer commented May 5, 2024

Generally sounds good to me!

Note isbn already appears in the faker.commerce module. We could probably make that a deprecated alias and move the implementation here

Id probably remove translator if it's just a random person name, doesn't add much (and if translator, why not illustrator, reviewer, etc)

Also not sure releaseYear is needed, too similar to faker.date.past() - which already has the ability to have a full release date, and to have the years be relative to current date.

Any methods we choose not to add to books for the MVP because there are too similar methods in other modules could still be referenced via see also and the module introduction. Eg if "author()" only has real authors we can cross reference faker.person.fullName() for people who want fake authors.

@cieslarmichal
Copy link
Contributor

Sounds good :) can't wait to start working on this!

@sinanaltaii
Copy link

Is this comming any time soon? :)

@cieslarmichal
Copy link
Contributor

I am ready to implement that, but we are waiting for version v9 to be done (major refactor)

@cieslarmichal
Copy link
Contributor

@xDivisionByZerox do you have any planned date for v9 release?

@xDivisionByZerox
Copy link
Member

@xDivisionByZerox do you have any planned date for v9 release?

Sorry for the late reply. We sadly dont have a fixed release date planed. We will put some more focus on this topic on thursdays, tho.
I understand that this might not be what you was hoping for. All I can do is thank you for your patience.

@cieslarmichal
Copy link
Contributor

cieslarmichal commented Jun 4, 2024

Ok I will wait.

@xDivisionByZerox
Copy link
Member

@cieslarmichal In today's meeting we made a new announcement regarding v9 progress. This means that you can technically start with the implementation of the book module if you wish to do so. Please keep in mind that the module itself will not be included in the v9 release, but can be the first thing for v9.1.

@cieslarmichal
Copy link
Contributor

cieslarmichal commented Jun 13, 2024

Hello, nice!
One question: should I create my branch from default branch (next) and then make a pull request to it?
Asking that because I can see there is a v9 branch and I am not sure about that situation with release.

I will start working on it this week 😄

@xDivisionByZerox
Copy link
Member

'next' is perfectly fine. The v9 branch is the latest v9 release. In this case v9.0.0-alpha.1.

@cieslarmichal
Copy link
Contributor

Ok thanks :)

@cieslarmichal
Copy link
Contributor

I am starting to work on it, planning to add following methods to MVP:

  • title
  • author
  • genre
  • publisher
  • format
  • series

I will also add a note that isbn can be generated with commerce module.

@cieslarmichal
Copy link
Contributor

created a PR with changes #2949

@ST-DDT ST-DDT linked a pull request Jun 15, 2024 that will close this issue
@ST-DDT ST-DDT added the m: book Something is referring to the animal module label Jun 15, 2024
@matthewmayer
Copy link
Contributor

matthewmayer commented Jun 16, 2024

Just commenting here to get some more visibility

Should author() return

Shakespeare, William

Or

William Shakespeare

@cieslarmichal
Copy link
Contributor

I will probably go with William Shakespeare as it is more readable and natural in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature m: book Something is referring to the animal module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants