Bug 95703 - [GTK] Print API missing documentation when generating gtkdoc
Summary: [GTK] Print API missing documentation when generating gtkdoc
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 523.x (Safari 3)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-03 17:18 PDT by Martin Robinson
Modified: 2012-09-04 11:32 PDT (History)
2 users (show)

See Also:


Attachments
Patch (4.98 KB, patch)
2012-09-03 18:06 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2012-09-03 17:18:24 PDT
It's a bit of a pain now to continually check the -unused.txt file when you are adding new documentation. We should print the missing documentation when running generate-gtkdoc.
Comment 1 Martin Robinson 2012-09-03 18:06:03 PDT
Created attachment 161953 [details]
Patch
Comment 2 Carlos Garcia Campos 2012-09-04 00:04:03 PDT
Comment on attachment 161953 [details]
Patch

Patch looks good to me, but it seems there are two independent changes, one is raising an exception when rebase is called before doc is generated, instead of just showing a message, I guess to make sure the script fails in such case, because the exception is not handled. And the other change is showing missing api when errors are reported. You could you split the patch before landing, please?
Comment 3 Martin Robinson 2012-09-04 05:55:33 PDT
(In reply to comment #2)
> (From update of attachment 161953 [details])
> Patch looks good to me, but it seems there are two independent changes, one is raising an exception when rebase is called before doc is generated, instead of just showing a message, I guess to make sure the script fails in such case, because the exception is not handled. And the other change is showing missing api when errors are reported. You could you split the patch before landing, please?

In this case, it was a small cleanup that I did while fixing the issue. If you feel it's serious enough that it needs to be in another patch, I'm happy to do that. Can I have a review in advance though?
Comment 4 Carlos Garcia Campos 2012-09-04 06:04:18 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 161953 [details] [details])
> > Patch looks good to me, but it seems there are two independent changes, one is raising an exception when rebase is called before doc is generated, instead of just showing a message, I guess to make sure the script fails in such case, because the exception is not handled. And the other change is showing missing api when errors are reported. You could you split the patch before landing, please?
> 
> In this case, it was a small cleanup that I did while fixing the issue. If you feel it's serious enough that it needs to be in another patch, I'm happy to do that. Can I have a review in advance though?

I already set r+
Comment 5 Martin Robinson 2012-09-04 07:17:47 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (From update of attachment 161953 [details] [details] [details])
> > > Patch looks good to me, but it seems there are two independent changes, one is raising an exception when rebase is called before doc is generated, instead of just showing a message, I guess to make sure the script fails in such case, because the exception is not handled. And the other change is showing missing api when errors are reported. You could you split the patch before landing, please?
> > 
> > In this case, it was a small cleanup that I did while fixing the issue. If you feel it's serious enough that it needs to be in another patch, I'm happy to do that. Can I have a review in advance though?
> 
> I already set r+

I'm still not sure if you consider the cleanup serious enough to split after my comment.
Comment 6 Martin Robinson 2012-09-04 07:19:28 PDT
Comment on attachment 161953 [details]
Patch

Oops. Guess I'll wait on this one. :)
Comment 7 Carlos Garcia Campos 2012-09-04 08:06:53 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > (From update of attachment 161953 [details] [details] [details] [details])
> > > > Patch looks good to me, but it seems there are two independent changes, one is raising an exception when rebase is called before doc is generated, instead of just showing a message, I guess to make sure the script fails in such case, because the exception is not handled. And the other change is showing missing api when errors are reported. You could you split the patch before landing, please?
> > > 
> > > In this case, it was a small cleanup that I did while fixing the issue. If you feel it's serious enough that it needs to be in another patch, I'm happy to do that. Can I have a review in advance though?
> > 
> > I already set r+
> 
> I'm still not sure if you consider the cleanup serious enough to split after my comment.

Not because it's serious, but because it's unrelated change, but I don't mind if you commit both in a single commit, I was just suggesting to split it before landing, no need to file new bug reports and patches. But still, feel free to cq+, I don't want to give you more work :-P
Comment 8 Martin Robinson 2012-09-04 08:33:54 PDT
Comment on attachment 161953 [details]
Patch

Okay. Thanks. I'll try to be more careful in the future.
Comment 9 WebKit Review Bot 2012-09-04 11:32:48 PDT
Comment on attachment 161953 [details]
Patch

Clearing flags on attachment: 161953

Committed r127475: <http://trac.webkit.org/changeset/127475>
Comment 10 WebKit Review Bot 2012-09-04 11:32:51 PDT
All reviewed patches have been landed.  Closing bug.