Ticket #532 (closed todo: fixed)

Opened 13 years ago

Last modified 11 years ago

Finish headerizer refactor

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

Description

t/codingstd/c_function_docs.t is using Parrot::Headerizer to extract function declarations, but tools/build/headerizer.pl is using its own (slightly different) copy of the same function.

Pick whichever one makes the most sense and use that in the PM, and use the PM version from the script.

Change History

  Changed 12 years ago by jkeenan

  • component changed from none to testing

  Changed 11 years ago by jkeenan

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

Since this ticket was filed, headerizer.pl has been moved from tools/build/ to tools/dev.

I do see that both t/codingstd/c_function_docs.t and tools/dev/headerizer.pl use the Parrot::Headerizer method extract_function_declarations. c_function_docs.t uses it in a straightforward manner:

55  my @function_decls = $headerizer->extract_function_declarations($buf);
56 
57   for my $function_decl (@function_decls) {
58 
59    my $escaped_decl = $headerizer->generate_documentation_signature($function_decl);
...

Its use in tools/dev/headerizer.pl is less straightforward. It's called within the main() subroutine:

    313 sub main {
...
    369         if ( $macro_match ) {
    370             @decls = $headerizer->extract_function_declarations( $source_code );
    371         }
    372         else {
    373             @decls = extract_function_declarations_and_update_source( $sourcefile );
    374         }

But if you look at the function in the else block, you see that extract_function_declarations appears within it as well:

     83 sub extract_function_declarations_and_update_source {
...
     90     my @func_declarations = $headerizer->extract_function_declarations( $text );

So I think we'll have to study the control flow in tools/dev/headerizer.pl before we can decide what to do.

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

Opened tt532_headerizer_refactor branch in Subversion to work on this ticket.

in reply to: ↑ 3   Changed 11 years ago by jkeenan

Replying to jkeenan:

Opened tt532_headerizer_refactor branch in Subversion to work on this ticket.

This branch was transformed into a git branch of the same name. I have been phalanxing tools/dev/headerizer.pl. Most of the code is now in either lib/Parrot/Headerizer/Object.pm or lib/Parrot/Headerizer/Functions.pm (names not set in stone yet). Tests for each of those modules may be found in t/tools/dev/headerizer/. Coverage stats for those tests may be found  on my web site.

Progess is being made.

kid51

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

I believe this branch is ready for merging into master. Please review the tt532_headerizer_refabor branch in git. You can test with make headerizer_tests.

I will merge this into master in 3 days unless I hear serious objections.

Thank you very much.

kid51

in reply to: ↑ 5   Changed 11 years ago by jkeenan

Replying to jkeenan:

s/tt532_headerizer_refabor/tt532_headerizer_refactor/

follow-up: ↓ 8   Changed 11 years ago by petdance

I'm not going to be able to look at this any time soon, Jim. I'm sorry.

in reply to: ↑ 7   Changed 11 years ago by jkeenan

Replying to petdance:

I'm not going to be able to look at this any time soon, Jim. I'm sorry.

Andy, thanks for getting back on this. I'll commit to master. When you get a chance, take a look at the POD and see if there's anything seriously wrong or easily improvable.

Thank you very much.

kid51

  Changed 11 years ago by jkeenan

Merged tt532_headerizer_branch into master and pushed. Deleted branch.

Before the final commit, I changed Parrot::Headerizer::Object back to Parrot::Headerizer and ran make fulltest on Linux/i386.

Closing ticket.

kid51

  Changed 11 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed
Note: See TracTickets for help on using tickets.