Bug 91758 - CodeGeneratorInspector.py is unnecessarily chatty
Summary: CodeGeneratorInspector.py is unnecessarily chatty
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Klein
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-19 10:40 PDT by Adam Klein
Modified: 2012-07-20 10:56 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.69 KB, patch)
2012-07-19 10:42 PDT, Adam Klein
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Klein 2012-07-19 10:40:16 PDT
CodeGeneratorInspector.py is unnecessarily chatty
Comment 1 Adam Klein 2012-07-19 10:42:46 PDT
Created attachment 153300 [details]
Patch
Comment 2 Peter Rybin 2012-07-19 13:43:59 PDT
The reason behind this message was to compensate over-simplified build rule. 

The generator has 7 output files and those files are written conditionally. Before writing output, generator tries to open destination file and does nothing, if the file already has correct content. This enables correct incremental build when generator is involved.
Since generator may or may not actually write its output even on build bot, the message to system.out is added.

I meant those messages to help track possible errors on the build bots (i.e. I see them meaningful).
On the other hand I'm fine with deleting them if it's the right thing to do.
Comment 3 Adam Klein 2012-07-19 14:29:22 PDT
(In reply to comment #2)
> I meant those messages to help track possible errors on the build bots (i.e. I see them meaningful).
> On the other hand I'm fine with deleting them if it's the right thing to do.

Do you see these messages as a short-term thing to make sure the code generator works (I see a previous version of this change was rolled out)?  If so, I'm fine with leaving them for a bit (say, a week or two) and then removing them. If you think they should always be emitted on the bots, then we'd want a different solution.
Comment 4 Peter Rybin 2012-07-19 14:35:55 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > I meant those messages to help track possible errors on the build bots (i.e. I see them meaningful).
> > On the other hand I'm fine with deleting them if it's the right thing to do.
> 
> Do you see these messages as a short-term thing to make sure the code generator works (I see a previous version of this change was rolled out)?  If so, I'm fine with leaving them for a bit (say, a week or two) and then removing them. If you think they should always be emitted on the bots, then we'd want a different solution.

I meant them to be permanent, cause I thought that 1 line per file is fair and is what build normally does, and 7 more doesn't change a lot.

Again, I'm fine with removing them. I believe that that change proved to be okay, so the excessive output could be removed at any moment.
Comment 5 Adam Klein 2012-07-19 14:39:10 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > I meant those messages to help track possible errors on the build bots (i.e. I see them meaningful).
> > > On the other hand I'm fine with deleting them if it's the right thing to do.
> > 
> > Do you see these messages as a short-term thing to make sure the code generator works (I see a previous version of this change was rolled out)?  If so, I'm fine with leaving them for a bit (say, a week or two) and then removing them. If you think they should always be emitted on the bots, then we'd want a different solution.
> 
> I meant them to be permanent, cause I thought that 1 line per file is fair and is what build normally does, and 7 more doesn't change a lot.

As I note in the ChangeLog, it's probably only noticeable when building with ninja, but there it's quite noisy (since ninja puts all build output besides errors on a single line).
Comment 6 Peter Rybin 2012-07-19 16:23:47 PDT
LGTM
Comment 7 Nico Weber 2012-07-20 10:01:09 PDT
Thanks!
Comment 8 WebKit Review Bot 2012-07-20 10:56:29 PDT
Comment on attachment 153300 [details]
Patch

Clearing flags on attachment: 153300

Committed r123233: <http://trac.webkit.org/changeset/123233>
Comment 9 WebKit Review Bot 2012-07-20 10:56:32 PDT
All reviewed patches have been landed.  Closing bug.