Skip to content
This repository

Document all the methods #9

Closed
schwern opened this Issue October 30, 2011 · 11 comments

4 participants

Michael G. Schwern Scott Walters Lars Dɪᴇᴄᴋᴏᴡ 迪拉斯 whitepearl
Michael G. Schwern
Collaborator

autobox::Core has a lot of undocumented methods. Document them.

You don't have to do them all in one go, there's a lot, but reference this issue (put "For #9" in your commit message) when you do. There is an existing branch for working in this, issue/9. Please work from that branch.

Test::Pod::Coverage may prove useful.

To clarify...

  • If autobox::Core is just wrapping around a core function, just list them.

For example, pop. Make a list for each data type. For bonus points, link each back to the perlfunc documentation, like L<pop|perlfunc/pop>.

  • If autobox::Core is providing a name for an operator, just document the name, what operator and any caveats.

For example, concat.

  • If autobox::Core is implementing its own, document that fully.

For example, trim.

  • If autobox::Core has caveats about a standard perl function, just document those caveats.

For example, split.

  • Method documentation should be placed under their appropriate =head3.

For example, pop should be listed after =head3 Array Methods but before another =head3.

  • Individual methods should be documented with a =head4.

Look at is_number for an example.

  • The existing non-standardized method documentation should be standardized (bonus)

For example, most of the docs under "Scalar String Related Methods". Information not relevant to the user should be removed, like "title_case", "center", "ltrim", "rtrim", and "trim" were taken from perl5i.

  • The existing =head3 should be standardized (bonus)

For example Scalar String Related Methods can be just String Methods.

  • The tests should not need to be touched.

Please ask lots of questions if you're not sure. I'm available on IRC at irc.perl.org as "schwern" on #gci and #perl5i.

Scott Walters
Owner

Some of these that other people besides myself have added do not have clear intentions to me.

The HASH get/at seem to be trying to do a slice but it doesn't seem to be working:

use autobox::Core;
use Data::Dumper;
use 5.010;

say { foo => 1, bar => 2, baz => 3 }->get('foo', 'baz');
3

Lars D implemented a times() method for scalars but there is no doc or comment and I don't see the point.

I'm going to need input from the authors of these bits of code on those so I don't "fix" them wrong.

Aside from that, this should be in pretty good shape now. Oops, forgot to reference the ticket number. I should know better. The commit is 26e9cc0: 26e9cc0

Tests are still lacking in places.

There is also still a lack of defined scope in this module. Personally, I'd like to take the methods for operators back out, with few exceptions.

Lars Dɪᴇᴄᴋᴏᴡ 迪拉斯
Collaborator

Lars D implemented a times() method

No, I applied a patch that changed the existing one.

there is no doc or comment and I don't see the point

Read the commit message of 5cc8df4. It references RT #32547. You yourself signed it off and closed the bug.

Michael G. Schwern
Collaborator

This task has been picked up by a Google Code-In student. I've added lots of detail.

whitepearl

From the details added for clarification, I get to learn that I have to modify the existing method definitions, properly list the functions, remove unnecessary documentation, standardize methods etc. of autobox::Core

Is anything else expected ?

Michael G. Schwern
Collaborator

@whiteperl Sorry. As happens so often in software development, the original issue wasn't fully specified. Nor was it written to be understood by someone outside the project. I know it looks like it was greatly expanded, but with the exception of the ones marked "bonus" it's clarifying how adding method docs is done. autobox::Core's documentation is a bit of a mess, so just following the surrounding style (which would normally serve you) doesn't work.

Commenting the tests (the .t files) in detail was not necessary to the task, I'm sorry you did all that work. I'll have to remove it as it's redundant clutter. :-/

I can't say I've specified everything, it's hard to write down all the assumptions. Usually with patches from any developer we go through a few rounds of corrections and clarifications. Often the process of working on a task provides more clarity. The important thing is to communicate with us here if you have doubts.

I'll totally understand if you want to drop the task. Your work wasn't wasted, we have a well specified task now and that has a lot of value.

Otherwise, and I hate to use the cliche, welcome to the real world of software development. :-/

PS Don't sweat the deadline, just keep chugging along.

whitepearl

I want to work on the task but I need help regarding the questions I have posted on the GCI task page.

Kindly help.

Michael G. Schwern
Collaborator

@whitepearl The GCI thing sometimes sends me email, sometimes it doesn't. Best thing is to talk about it here so I'll be sure to see it, other people on the project can see it, and it's recorded.

Also if you turn your work into a pull request it will be easier to track and work on. It should be a small matter of hitting the "pull request" button while looking at your changes on Github. Then we can commit directly on your code. Any additional changes you commit and push to that branch will show up in the pull request.

Scott Walters
Owner

Hi whitepearl, Schwern,

I made a pass through the docs not long ago in response to this ticket. I thought that the situation was much improved after that but I haven't heard any opinions one way or the other.

A few thoughts. I've left "XXX" littered around the code and docs as a sort of "TODO" or "FIXME". The trim() method Schwern mentioned has one of these in its docs:

This is redundant and subtly different from C XXX.

That particular case calls for reading through perl5i documentation, figuring out where in there trim comes from and how it differs from the one here, then either documenting the difference or else changing autobox::Core to match what is happening there.

Some places in the docs, I've written that "X appears to be Y". In those cases, a person could either figure out if that is the case through some means I've failed to identify or else ask the original author of the code what their intention was and then doc that.

There are large swaths of content that could be deleted from there, but if you do that, please coordinate with me. I'd like to archive them elsewhere and perhaps reference that in the SEE ALSO section. The chunk of manuscript from Perl 6 Now: The Core Ideas Illustrated with Perl 5 comes to mind. You might ask here or on IRC in #perl5i for opinions on that and other edits.

Thank you for your interest in what's essentially janitorial work.

Michael G. Schwern schwern referenced this issue from a commit November 27, 2011
Shivani Maheshwari For #9 cc007ff
Michael G. Schwern schwern referenced this issue from a commit November 28, 2011
Michael G. Schwern Fixup the string methods.
* Detabify
* Explain how string methods work
* Move eq() out of the numeric methods
* Fix typos
* Put trim, ltrim and rtrim together
* Document ltrim and rtrim in terms of trim
* Cut back on docs redundant with perlfunc
* No need to repeat the method name at the start of the method description.
* Move the list of built in methods to the top of the section for consistency

For #9
c11c541
Michael G. Schwern schwern referenced this issue from a commit November 28, 2011
Michael G. Schwern Fixup the numeric methods.
* A lot are string methods
* Remove docs redundant with perlop

For #9
1c34867
Michael G. Schwern schwern referenced this issue from a commit November 28, 2011
Michael G. Schwern Most of the existing reference method docs are unnecessary.
* quotemeta has nothing to do with references
* ref docs were redundant

For #9
505a28b
Michael G. Schwern schwern referenced this issue from a commit November 28, 2011
Michael G. Schwern Fixup array methods.
* Move generally useful information about contextual returns to the front
* Detabify
* Compress aliased methods

For #9
dd876d4
Michael G. Schwern schwern referenced this issue from a commit November 28, 2011
Michael G. Schwern Fixup the hash methods.
For #9
5c1ba28
Michael G. Schwern schwern referenced this issue from a commit November 28, 2011
Michael G. Schwern Fixup the code methods.
times is commented out in the code, so we're not documenting it.

For #9
5e74d0f
Michael G. Schwern
Collaborator

@scrottie I would suggest moving any XXX into issues.

The docs may previously have mentioned everything, I'm not sure. Now they do it in a consistent way.

I'm not sure what content you want to archive or where exactly, but we're just doing the method listing in this issue. The surrounding documentation can go another time.

I'd say next step before releasing with this documentation is to decide if we WANT to document everything. There's some marginally useful stuff in there that's already better served by built ins. Or stuff with weird or redundant names. But that's another issue.

@whitepearl I decided it would be faster to fix all the little nits than to write you a review. :) A lot of them are my personal editing choices.

It's not perfect, but it's much better than it was. I think this is ok to close up?

whitepearl

@Schwern thank you for fixing the problems.

Please let me know the final decision as soon as possible as I have to leave out of town within 8 hours. And then I may not have Internet with me.

I would be highly obliged for the same.

Michael G. Schwern
Collaborator

Just merged this in.

Michael G. Schwern schwern closed this December 05, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.