Bug 95703 – [GTK] Print API missing documentation when generating gtkdoc

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

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.

[Martin Robinson]

Created attachment 161953 [details]
Patch

[Carlos Garcia Campos]

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?

[Martin Robinson]

(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?

[Carlos Garcia Campos]

(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+

[Martin Robinson]

(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.

[Martin Robinson]

Comment on attachment 161953 [details]
Patch

Oops. Guess I'll wait on this one. :)

[Carlos Garcia Campos]

(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

[Martin Robinson]

Comment on attachment 161953 [details]
Patch

Okay. Thanks. I'll try to be more careful in the future.

[WebKit Review Bot]

Comment on attachment 161953 [details]
Patch

Clearing flags on attachment: 161953

Committed r127475: <http://trac.webkit.org/changeset/127475>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.