Possibility of moving to YARD-style documentation in README

[stephendolan]

I find some of the documentation and READMEs in this project relatively difficult to use. For example, in Faker::Lorem:

# Optional arguments: word_count=4, supplemental=false, random_words_to_add=6
# The 'random_words_to_add' argument increases the sentence's word count by a random value within (0..random_words_to_add).
# To specify an exact word count for a sentence, set word_count to the number you want and random_words_to_add equal to 0.
# By default, sentences will have a random number of words within the range (4..10).
Faker::Lorem.sentence #=> "Dolore illum animi et neque accusantium."
Faker::Lorem.sentence(3) #=> "Commodi qui minus deserunt sed vero quia."
Faker::Lorem.sentence(3, true) #=> "Inflammatio denego necessitatibus caelestis autus illum."
Faker::Lorem.sentence(3, false, 4) #=> "Aut voluptatem illum fugit ut sit."
Faker::Lorem.sentence(3, true, 4) #=> "Accusantium tantillus dolorem timor."

There's nothing here specifying what the supplemental parameter does, and there's also no solid indication in the examples. Visiting the code itself isn't any more helpful:

def sentence(word_count = 4, supplemental = false, random_words_to_add = 0)
  words(word_count + rand(random_words_to_add.to_i), supplemental).join(' ').capitalize + locale_period
end

I ended up being able to most easily find the purpose of this parameter on this GitHub issue.

What I would expect in the README:

# Optional arguments: word_count=4, supplemental=false, random_words_to_add=6
# Faker::Lorem.sentence(word_count = 4, supplemental = false, random_words_to_add = 6)
#
# @param [Integer] word_count the number of words to include in the generated sentence.
# @param [Boolean] supplemental whether or not to pull from an additional,  supplemental set of Lorem text.
# @param [Integer] random_words_to_add a number of words between 0 and this value will be added to the generated sentence.
Faker::Lorem.sentence #=> "Dolore illum animi et neque accusantium."
Faker::Lorem.sentence(3) #=> "Commodi qui minus deserunt sed vero quia."
Faker::Lorem.sentence(3, true) #=> "Inflammatio denego necessitatibus caelestis autus illum."
Faker::Lorem.sentence(3, false, 4) #=> "Aut voluptatem illum fugit ut sit."
Faker::Lorem.sentence(3, true, 4) #=> "Accusantium tantillus dolorem timor."
Faker::Lorem.sentence(3, false, 4) #=> "Aut voluptatem illum fugit ut sit."
Faker::Lorem.sentence(3, true, 4) #=> "Accusantium tantillus dolorem timor."

What I would expect in the code:

# Generate a sentence with Lorem Ipsum text.
#
# @param [Integer] word_count the number of words to include in the generated sentence.
# @param [Boolean] supplemental whether or not to pull from an additional,  supplemental set of Lorem text.
# @param [Integer] random_words_to_add a number of words between 0 and this value will be added to the generated sentence.
#
# @return [String] a random sentence generated from Lorem Ipsum text.
def sentence(word_count = 4, supplemental = false, random_words_to_add = 0)
  words(word_count + rand(random_words_to_add.to_i), supplemental).join(' ').capitalize + locale_period
end

I wanted to start a discussion here to see whether or not there's an interest in beginning a larger effort to improve this library's documentation. It is such an excellent source of data that it is a shame to see any of its cool options hidden.

It's also entirely possible that I've missed an additional source of documentation somewhere :)

[Zeragamba]

YARD documentation is supported in RubyMine, and I think VS Code as support for it as well.

[Zeragamba]

I've started to add YARD style docs. an example can be seen here:

• https://www.rubydoc.info/github/SpyMaster356/faker/382145/Faker/Blockchain/Bitcoin
• https://www.rubydoc.info/github/SpyMaster356/faker/382145/Faker/Blockchain/Tezos

[stympy]

I really like this. Who's willing to take up the challenge of converting all the docs? :)

[stephendolan]

I don't think that there necessarily needs to be a conversion effort to move all of the documentation immediately, though that would certainly be the dream :).

I was more envisioning a discussion here about which YARD documentation elements should be required, how/where to go about documenting those in the CONTRIBUTING.md, and then how to go about enforcing those documentation requirements for every new pull request before it is allowed to be merged.

[Zeragamba]

I agree with that, we can ask existing/new pull requests that haven't been merged yet to include YARD doc blocks.

An full example would be:

##
# @return [string] A hex string
#
# @param length [integer] Length of the string
# @param upcase [boolean] Upcase the letters 
#
# @example Faker::Example.hex
#   "d6964ad8b71d46a62e5e64cd00b9e476"
#   "955b1fdf9971cda5745604ab081cc61d"
#   "73d3dc6fa0ba67911024d61eb6c1e382"
# @example Faker::Example.hex(length: 10)
#   "170b43193e"
#   "181450a4ae"
#   "d7b0a72da6"
# @example Faker::Example.hex(upcase: true)
#   "4BE0CB215BE62A06306BF21872EA9603"
#   "8A73D1E0BB69E3A5388EA0E340A0B0D9"
#   "F0C078EECFFCA394336F93B54D6CDE24"
#
# @faker.version next
def hex(length: 32, upcase: false)

[Zeragamba]

I know that YARD also allows for us to specify our own CSS for the documentation, so we could so some fancy things with that, but i haven't yet delved into that.

[stephendolan]

A few comments on the proposed format provided by @SpyMaster356 :

1. @param length [integer] Length of the string should probably be @param length [Integer] the length of the string (note the lower case description and capitalized data type). Making explicit where we wish to depart from the style laid out in the Tags documentation for YARD will be necessary to maintain consistency.

2. What's the intended use of the @faker.version tag? Just want to make sure it's actually relevant for the people who will be reviewing the documentation. Personally, what version a function was introduced in wouldn't be of interest to me as an end user.

3. I firmly believe in having a summary statement at the top of all YARD docs, even if that summary statement is repetitive. I would probably have added something to the top of your comment along the lines of "Produces a random string of hexadecimal characters.". I find that it makes the end documentation even more consistent and easier to parse.

Without summary:

With summary:

[Zeragamba]

1. I didn't know there was a suggested format for tags. I think we should adhere to that as much as possible . However, when i was poking around with it, i was running into the issue where [String] was resolving to Faker::String instead of standard lib's String. Do you know if there is a way to force Yard to pick the standard lib one?

2. In the current markdown docs, there is often a line "Available since version 1.9.0" or similar, and i think it would be good to keep that for users that have locked down their version of faker.

3. Nearly all the methods are going to be single line "produce a random this", "produce a random that", so i don't know if a summary is completely needed. We could drop the description on the @return tag to remove the duplication.

[stephendolan]

I'm not sure about the String resolution issue off the top of my head, but maybe prefixing it like ::String would pull it out of the Faker namespace.

Everything else makes perfect sense to me and I agree with your approach, for whatever that's worth to the group :)

[Zeragamba]

So an updated example:

##
# Produces a random hexadecimal string
#
# @return [string]
#
# @param length [integer] Length of the string
# @param upcase [boolean] Upcase the letters 
#
# @example Faker::Example.hex
#   "d6964ad8b71d46a62e5e64cd00b9e476"
#   "955b1fdf9971cda5745604ab081cc61d"
#   "73d3dc6fa0ba67911024d61eb6c1e382"
# @example Faker::Example.hex(length: 10)
#   "170b43193e"
#   "181450a4ae"
#   "d7b0a72da6"
# @example Faker::Example.hex(upcase: true)
#   "4BE0CB215BE62A06306BF21872EA9603"
#   "8A73D1E0BB69E3A5388EA0E340A0B0D9"
#   "F0C078EECFFCA394336F93B54D6CDE24"
#
# @faker.version next
def hex(length: 32, upcase: false)

[Zeragamba]

One thing that we should discuss is the number of examples that are required.

A few ideas for that:

1. Require 1 example for each usage
2. Require 3 examples for each usage (unless it's always the same result, eg Faker::Placeholdit.image)
3. Require 3 examples for the primary usage, and 1 for each of the secondary usages

[stympy]

I'm fine with requiring just one example and encouraging examples for optional arguments.

[stephendolan]

I don't think anything more than 1 example showing the main use case should be required (of course, the more the better). The @param and @return tags should be detailed enough to inform users about the various uses on their own, and is where the focus should be placed during documentation review.

[Zeragamba]

For me, I think each usage should have at least one example, as that can speak more about what will be generated. But generally @param and the summary should be enough in most cases.

[Zeragamba]

Oh cool, Rubocop has a documentation cop, we could make use of that once all the methods are documented.

[stephendolan]

When utilizing YARD for documentation, I've found the yard stats command that comes with the gem to be very helpful as well. You can use some simple command line tools to ensure that your documentation percentage doesn't drop.

It seems like we've got enough of a start here to open up a pull request. Does it make sense to include these guidelines in CONTRIBUTING.md, or is it worth having something like a DOCUMENTATION.md document that can be linked from CONTRIBUTING.md?

[Zeragamba]

To me, having the new documentation style as part of CONTRIBUTING.md makes the most sense to me. And looking at the file, it actually doesn't mention documentation at all >.<

Adding the check to ensure that the documentation level doesn't drop (btw, currently sits at 7% excluding lib/faker/deprecate) would be good to have from the start.

Sounds good to you @stympy?

[stympy]

That works for me.

[Zeragamba]

@stephendolan i've already started a bit on my fork here: https://github.com/SpyMaster356/faker/tree/yard-powered-docs

[stephendolan]

Just so I'm clear @SpyMaster356 , are you submitting the initial pull request, or should I?

[Zeragamba]

We could work off of my fork as it's already got Yard added, and some basic setup done. Up to you, as you opened this thread.

On another note, using ::String for the type didn't fix the name-spacing issue, and it looks like even YARD has a similar issue: https://www.rubydoc.info/gems/yard/String

[Zeragamba]

Should i open the pull request @stephendolan?

[stephendolan]

I'm happy to open one, @SpyMaster356 . It will just be the week of March 8th before I can devote the time to getting it done correctly. If we'd like to see progress before then, someone else will have to open the PR.

We've got a big release at work this week, followed by a big vacation for me :D.

[Zeragamba]

Have fun on the vacation. I'll open a PR for the stuff i have on my fork.