Bug 91758 – CodeGeneratorInspector.py is unnecessarily chatty

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Adam Klein]

CodeGeneratorInspector.py is unnecessarily chatty

[Adam Klein]

Created attachment 153300 [details]
Patch

[Peter Rybin]

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.

[Adam Klein]

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

[Peter Rybin]

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

[Adam Klein]

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

[Peter Rybin]

LGTM

[Nico Weber]

Thanks!

[WebKit Review Bot]

Comment on attachment 153300 [details]
Patch

Clearing flags on attachment: 153300

Committed r123233: <http://trac.webkit.org/changeset/123233>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.