-
Notifications
You must be signed in to change notification settings - Fork 21
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
Document unit tests (issue #6) #7
base: master
Are you sure you want to change the base?
Conversation
keithrbennett
commented
Feb 12, 2021
- adds documentation for unit test methods and constants
- adds an explanatory note to the benchmark.rb file
- adds an IMPLEMENTATION.md file with an explanation of label width handling
- fixes a misspelling in a test method name
…dle differently (GH issue ruby#6).
In general, test suites for the Ruby standard library do not document what is being tested to this level. That could be considered a good thing (less noise when updating tests) or bad thing (hard to understand what is being tested) depending on your point of view, but this change would make benchmark inconsistent with most of the rest of the standard library. If you think we should start documenting standard library tests like this, you should probably create a Misc ticket on the Ruby bug tracker, and add it to the list of topics to be discussed at the next developer meeting. I think the change made in |
Hi, Jeremy. Regarding the test documentation, it took me a while to figure
out everything that was going on; there was often more information that
needed to be known than could be fit in the test name, and nuances that
were not readily apparent. I figured the documentation would be helpful to
someone else wanting to understand the tests and even the lib code it
tested.
Personally, if it were me, I would welcome a well documented file even if
it were inconsistent with less- or undocumented code elsewhere in the code
base, assuming the documentation was truly helpful and not mere proforma
verbosity.
Although documentation can get stale quickly, this code has been very
stable over the years.
That said, I will follow your direction and make a separate PR for the test
name fix and the lib file. Regarding the documentation in the new
IMPLEMENTATION.md file, I will assume you don't want to keep it or move it
somewhere else unless you tell me otherwise.
Thanks for looking into this.
…On Mon, Mar 1, 2021 at 5:17 PM Jeremy Evans ***@***.***> wrote:
In general, test suites for the Ruby standard library do not document what
is being tested to this level. That could be considered a good thing (less
noise when updating tests) or bad thing (hard to understand what is being
tested) depending on your point of view, but this change would make
benchmark inconsistent with most of the rest of the standard library. If
you think we should start documenting standard library tests like this, you
should probably create a Misc ticket on the Ruby bug tracker, and add it to
the list of topics to be discussed at the next developer meeting.
I think the change made in lib is good, and fixing misspelled test names
is also a good change to make. So those changes we could easily merge
separately if you submit a separate pull request for them.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#7 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAG56SBJBUV5C276XZSINTTBQG7BANCNFSM4XP7QA5A>
.
|