Ticket #324 (closed cage: fixed)

Opened 6 years ago

Last modified 4 years ago

Write function documentation

Reported by: cotto Owned by: jkeenan
Priority: normal Milestone:
Component: docs Version:
Severity: medium Keywords: docs
Cc: Language:
Patch status: Platform:

Description

The following is a copy/paste from RT #48260, which this ticket supercedes.

There are MANY functions within the Parrot repository which as yet aren't documented. These need to be documented, please! Each needs an appropriate description of what the function does, and the meaning of any arguments.

Attachments

revised.c_function_docs.47551.diff Download (29.2 KB) - added by jkeenan 4 years ago.
diff between cfunctionsdocs branch and trunk, with revisions suggested by Util++
untitled-part.html Download (2.8 KB) - added by mikehh 4 years ago.
Added by email2trac

Change History

follow-up: ↓ 4   Changed 6 years ago by whiteknight

  • status changed from new to assigned
  • owner set to whiteknight
  • keywords docs added
  • milestone set to 1.0

Most of the undocumented functions I saw last time I looked at this ticket were in languages/*. With languages leaving the nest soon, the number of undocumented functions in the repo will be reduced dramatically.

Almost all of the functions in src/* are documented. All of the functions in src/pmc/* and src/ops/* are (last I checked) There is a lot of documentation missing in compilers/imcc/* and compilers/pirc/* (last I checked, kjs may have added more since then, and I haven't wanted to go monkey around in that directly while he's working)

I'll try to get a list together later today.

  Changed 6 years ago by coke

There have been competing ways to solve this problem.

A test was created to squawk about undocumented functions.

Then someone added in "document me" stub functions which "satisfied" the test. This, IMO, was a waste of cycles, and completely defeats the purpose of the test.

Let's find any functions that still have the old RT reference and say document me, remove them, and work on making the test pass. Once it passes, we can enable it as a regularly run test.

  Changed 6 years ago by coke

  • summary changed from [TODO] [C] Write function documentation to Write function documentation

in reply to: ↑ 1   Changed 6 years ago by kjs

The problem is not "please document them", but instead there's many functions of which it's just not clear what it does to the average parrot hacker (well, maybe it's just me), what the pre/postconditions are, what each of the arguments is ("int i" kind of declarations).. I think otherwise there wouldn't be so many undocumented functions.

Replying to whiteknight:

Most of the undocumented functions I saw last time I looked at this ticket were in languages/*. With languages leaving the nest soon, the number of undocumented functions in the repo will be reduced dramatically. Almost all of the functions in src/* are documented. All of the functions in src/pmc/* and src/ops/* are (last I checked) There is a lot of documentation missing in compilers/imcc/* and compilers/pirc/* (last I checked, kjs may have added more since then, and I haven't wanted to go monkey around in that directly while he's working)

That must have been a long time ago then.. please check [compilers/pirc] again ;-)

  Changed 6 years ago by coke

I removed all the non-documentation headers, and started bring t/codingstd/c_function_docs.t in line with not-so-recent changes with function annotations.

"2103 functions lacking documentation" - it will complain if the proper =item signature is missing entirely, and give a different diagnostic if the signature is present but no documentation exists.

What should the documented signature look like with annotations? I stripped out the leading PARROT_ annotations, and ARGIN(), ARGMOD() from the test, but I'm unsure about things like SHIM() and SHIM_INTERP - how should we show those in the function docs?

  Changed 6 years ago by coke

I made some best-guess effort (infinoid++ for better guessing) as to what these should look like, and switched around the test file so that any files that are passing can be specifically untodo'd, so they don't backslide. (Also, new files will not be skipped, so new files will need to be documented to pass). There's a DATA section listing the ones still TODO here; the ones actually missing documentation are near the bottom; most of the others just haven't kept argument listings, so fixing up the signatures makes them pass.

If you run the test against a single file, it will show you the missing docs, along with the expected signature, making it easy to cut and paste the fix.

The test distinguishes in its diagnostic output if the signature is missing (could be there with the wrong args!) or if ONLY the signature is there, but no docs. (This replaces the old "not documented yet" cruft.)

  Changed 6 years ago by coke

As of r37826, down to 1256 functions failing this test in 101 files.

  Changed 6 years ago by coke

Infinoid++ pointed out (with a commit) that 'make headerizer' updates the function docs in a way that was incompatible with the style we were checking.

Updated headerizer and the doc check to use the same algorithm, reran headerizer, updated the test file to skip the appropriate files, down to 540 functions checked without docs.

  Changed 6 years ago by allison

  • milestone changed from 1.1 to 1.2

follow-up: ↓ 11   Changed 6 years ago by whiteknight

Do we have an updated list (or a good way for me to make a list) of the undocumented functions?

in reply to: ↑ 10   Changed 6 years ago by Infinoid

Replying to whiteknight:

Do we have an updated list (or a good way for me to make a list) of the undocumented functions?

At the end of t/codingstd/c_function_docs.t, under the DATA, you'll find a big list of files containing undocumented functions. These are currently TODOed. See r37817 for Coke++'s implementation and rationale.

If you unTODO them, I think that test will give you a list.

  Changed 6 years ago by allison

  • milestone 1.2 deleted

Removing the 1.2 milestone as this is an ongoing task.

follow-up: ↓ 14   Changed 4 years ago by whiteknight

  • owner changed from whiteknight to kid51
  • status changed from assigned to new

in reply to: ↑ 13   Changed 4 years ago by jkeenan

  • status changed from new to assigned
  • owner changed from kid51 to jkeenan

Replying to whiteknight:

whiteknight, for purposes of Trac I am jkeenan. kid51 is my entity on IRC and use.perl.org.

But thanks for spotting this old ticket, which is applicable to the task at hand.

kid51

  Changed 4 years ago by jkeenan

Final call for review of the cfunctionsdocs branch in our Subversion repository. See attachment for updated diff against trunk at branch point.

kid51

Changed 4 years ago by jkeenan

diff between cfunctionsdocs branch and trunk, with revisions suggested by Util++

Changed 4 years ago by mikehh

  Changed 4 years ago by mikehh

Hi,

I did document the missing functions etc. on trac at
http://trac.parrot.org/parrot/wiki/CFunctionDocs
with links for MissingCFunctionDocs and BoilerplateOnlyDocs.

I haven't updated this recently (last at r43470) but will sort this out
after the branch merge tomorrow.

One thing to note:

t/codingstd/c_function_docs.t uses lib/Parrot/Headerizer.pm to check for and
generate expected signatures,
this uses a line length of 80 characters, whereas technically pod should
have a maximum of 78 chars.

This does not seem to have caused any problems with parrot at the moment, so
I don't think I want to mess with this.

Cheers, Michael (mikehh)


On 28 June 2010 23:10, Parrot <parrot-tickets@lists.parrot.org> wrote:

> #324: Write function documentation
>
> --------------------+-------------------------------------------------------
>  Reporter:  cotto   |       Owner:  jkeenan
>     Type:  cage    |      Status:  assigned
>  Priority:  normal  |   Milestone:
> Component:  docs    |     Version:
>  Severity:  medium  |    Keywords:  docs
>     Lang:          |       Patch:
>  Platform:          |
>
> --------------------+-------------------------------------------------------
> Changes (by jkeenan):
>
>  * owner:  kid51 => jkeenan
>  * status:  new => assigned
>
>
> Comment:
>
>  Replying to [comment:13 whiteknight]:
>
>  whiteknight, for purposes of Trac I am jkeenan. kid51 is my entity on IRC
>  and use.perl.org.
>
>  But thanks for spotting this old ticket, which is applicable to the task
>  at hand.
>
>  kid51
>
> --
> Ticket URL: <https://trac.parrot.org/parrot/ticket/324#comment:14>
> Parrot <https://trac.parrot.org/parrot/>
> Parrot Development
> _______________________________________________
> parrot-tickets mailing list
> parrot-tickets@lists.parrot.org
> http://lists.parrot.org/mailman/listinfo/parrot-tickets
>



-- 
Michael H. Hind

Cell: +44 (0) 7877 224 745

untitled-part.html Download

Changed 4 years ago by mikehh

Added by email2trac

follow-up: ↓ 18   Changed 4 years ago by jkeenan

cfunctionsdocs branch was merged into trunk at r47917.

mikehh: I didn't see your post to this TT until after the merge. So if there is anything that needs fixing re your post (and I don't think there is), you can make changes directly to trunk.

From this point forward, if you add C functions you will have to immediately add documentation of those functions (best) or add the file to the __DATA__ section in t/codingstd/c_functions_docs.t and promise-cross-your-heart to add to the documentation (far from the best).

Will close ticket in 48 hours, assuming no complaints.

Thanks to mikehh, Util, doughera, Coke and all others who contributed to this effort.

kid51

in reply to: ↑ 17   Changed 4 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

Replying to jkeenan:

Will close ticket in 48 hours, assuming no complaints.

No complaints. Closing ticket.

Note: See TracTickets for help on using tickets.