Re: [webkit-dev] style-queue entering beta
Maybe in the case of failure, provide a link to further information? Kenneth On Sun, Nov 29, 2009 at 3:44 AM, Maciej Stachowiak m...@apple.com wrote: On Nov 28, 2009, at 10:53 AM, Adam Barth wrote: On Sat, Nov 28, 2009 at 10:21 AM, Maciej Stachowiak m...@apple.com wrote: On Nov 28, 2009, at 2:21 AM, Adam Barth wrote: 1) Adding an extra flags is going to cause confusion. The style-queue does not add any flags to Bugzilla. Instead of storing it's state in Bugzilla flags (like commit-queue does), we built an AppEngine web service to hold the queue's persistent state. Instead of indicating style errors with a negative flag, the bot adds a comment to the bug. It does seem that flags are noisy in an unappealing way, but it would be much better (IMO) to store this information in the bugzilla database instead of externally. Is there any way we can do that? From an implementation point of view, either way is easy now that we've got the infrastructure built. It's a question of which you'd prefer. Could we make a flag that is hidden in the default UI, or use specially formatted comments that the bot knows how to recognize? From my point of view, a hidden flag could work. We considered specially formatted comments, but that would make the bot more chatty. For example, if the style-queue has some internal error that prevents it from processing the patch, it wants to remember that, but it doesn't want to spam the bug with that information. I'm not sure representing all the state in Bugzilla is necessary. We should represent the most interesting state (e.g., pass / fail) there. For the rest, we can have a dashboard similar to build.webkit.org that shows the status of various patches before they are committed. I've started sketching something rough here: http://webkit-commit-queue.appspot.com/static/details.html Pass/fail status sounds fine to me. I agree that an error by the bot should not be highly visible in the bug, as that is not as useful to the review and submitter as Pass or Fail and Here's the Mistakes. You can imagine clicking those squares to see the full log of what happened. For example, if the build failed on Qt, you might want to see the full output of build-webkit --qt, but we don't need to post all that to Bugzilla. The comment might just say: Looks like a decent design. Patch 86912 did not build on Qt. Build log: http://webkit-commit-queue.appspot.com/patch-status/build-queue-qt/86912/all At a higher level, I'm sympathetic to Mark's concerns about what the system will look like when we have a number of bots. Bugzilla flags work well for receiving input from humans. There are lots of choices for how to present output. For example, another option is to have a bunch of colored squares next to each attachment that represent that patch's row on the dashboard. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- Kenneth Rohde Christiansen Technical Lead / Senior Software Engineer Qt Labs Americas, Nokia Technology Institute, INdT Phone +55 81 8895 6002 / E-mail kenneth.christiansen at openbossa.org ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] HTML Layers above plugin
Hi, I am currently investigating the possibility of using a web container for an HTPC system. As part of the design I am looking at is to use the multiple tab views to embed the necessary elements. One would concentrate on the menu and another on the embedded player (via plugin). For the view showing the plug-in I have two approaches that I am looking at, where is either work with with plug-in developers to add the necessary chrome needed to display the play various information or to somehow overlay 'controls' in HTML. If I do create a layer with a z-index above the video plugin it will get obstructed by the plugin. On the other hand I am able to hide the layer with the plug-in without any issue. Is there are a way to draw HTML above the plugin? If not is there any technical reason why something like this could not be implemented? André-John ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
On Sat, Nov 28, 2009, Adam Barth wrote: One of the bots that Eric and I have been working on is about to come online. This bot is a style bot that runs check-webkit-style on patches that have been nominated for review. This seems like a good effort, thanks. A couple minor comments below on the first question in your FAQ: == Frequently Asked Questions == Q1: If the style-queue doesn't complain, does that mean my patch has correct style? A1: Unfortunately, no. First of all, check-webkit-style has false negatives. It seems like this answers the different question, If the style-queue complains, does that mean my patch has incorrect style? Hopefully, the script will improve over time, but it will never be perfect. Can you elaborate on this? For example, are you saying there is a basic reason that the script will always have bugs? Without knowing too much about the script, it seems like it wouldn't be too hard to at least make the false negatives go away. Or are you simply saying that the guidelines and script will never fully capture what we mean by correct style? Second, the style-queue is only able to check patches that successfully download and apply to top-of-tree. If your patch does not apply to top-of-tree (e.g., because it depends on another patch), then style-queue won't say anything. Thanks, --Chris ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
On Sun, Nov 29, 2009 at 9:26 AM, Chris Jerdonek chris.jerdo...@gmail.com wrote: On Sat, Nov 28, 2009, Adam Barth wrote: A1: Unfortunately, no. First of all, check-webkit-style has false negatives. It seems like this answers the different question, If the style-queue complains, does that mean my patch has incorrect style? Yes, that too. I would call that a false positive, but you're right that check-webkit-style probably has both kinds of bugs. Hopefully, the script will improve over time, but it will never be perfect. Can you elaborate on this? For example, are you saying there is a basic reason that the script will always have bugs? Without knowing too much about the script, it seems like it wouldn't be too hard to at least make the false negatives go away. Or are you simply saying that the guidelines and script will never fully capture what we mean by correct style? Does this mean you're volunteering to remove all the false positives and false negatives? :) One basic reason the script isn't perfect is that it's doesn't have a full C++ / Objective-C++ parser. I think it uses regular expressions. Anyway, I just wanted to set expectations that the tool might not be perfect. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] HTML Layers above plugin
On Sun, Nov 29, 2009 at 7:11 AM, Andre-John Mas aj...@sympatico.ca wrote: I am currently investigating the possibility of using a web container for an HTPC system. As part of the design I am looking at is to use the multiple tab views to embed the necessary elements. One would concentrate on the menu and another on the embedded player (via plugin). For the view showing the plug-in I have two approaches that I am looking at, where is either work with with plug-in developers to add the necessary chrome needed to display the play various information or to somehow overlay 'controls' in HTML. If I do create a layer with a z-index above the video plugin it will get obstructed by the plugin. On the other hand I am able to hide the layer with the plug-in without any issue. Is there are a way to draw HTML above the plugin? If not is there any technical reason why something like this could not be implemented? This is a web development question and probably not appropriate for this list, but: - If the plugin is windowed, consulting your favorite search engine for [iframe shim] will put you on the right track. - If the plugin supports windowless mode, it layers like a normal HTML element. E.g. see [flash wmode] for discussion of this for Flash. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Platform API for Uniscribe/ComplexTextController?
Hi all, The wx port is starting to look at getting proper support for complex text, and one of the first places I started looking at was the Win and Mac port implementations. I noticed that the complex text code in FontWin.cpp and FontComplexTextMac.cpp is largely the same, passing the heavy lifting down to their respective complex text controllers, and I actually wondered if they could be merged if there were some tweaks to the APIs to make them compatible. That led me to wonder if we couldn't make ComplexTextController one of our platform classes and have USE() defines to determine the implementation. Then we could move the drawComplexText, floatWidthForComplexText, and offsetForPositionForComplexText into Font.cpp guarded by that USE() define. The wx port can provide the native font handles the Win/Mac controllers need, so it'd really be great if we could just add these into our build to get complex text support working without having to implement the complex text methods differently for each platform. BTW, I actually already did get UniscribeController compiling, actually, but on Windows I had to have the build script copy it to platform/graphics/wx because MSVC implicitly puts the current directory first on the path, which was causing it to pick up platform/graphics/win/FontPlatformData.h instead of the wx one when compiling that file. :( So that's something else that would need to be addressed if ports can start sharing these files. Thanks, Kevin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
On Sun, Nov 29, 2009 at 9:35 AM, Adam Barth aba...@webkit.org wrote: On Sun, Nov 29, 2009 at 9:26 AM, Chris Jerdonek chris.jerdo...@gmail.com wrote: On Sat, Nov 28, 2009, Adam Barth wrote: Hopefully, the script will improve over time, but it will never be perfect. Can you elaborate on this? For example, are you saying there is a basic reason that the script will always have bugs? Without knowing too much about the script, it seems like it wouldn't be too hard to at least make the false negatives go away. Or are you simply saying that the guidelines and script will never fully capture what we mean by correct style? Does this mean you're volunteering to remove all the false positives and false negatives? :) I was hoping to work on the script eventually, which is partly why I asked for elaboration. All that I meant above is that one could potentially disable (for the bot) the style tests that report false violations, or else reduce their confidence score. That way, if the style bot flags a patch, it is guaranteed to be meaningful without looking at the details of the report. This can only be done, though, if the problems with the script are not so basic that they affect most or many of the tests. (The reverse is not as straightforward, though. It does not seem as easy to change the script -- in a useful way -- so that if it reports that a patch has met the guidelines, then the patch really meets the guidelines.) One basic reason the script isn't perfect is that it's doesn't have a full C++ / Objective-C++ parser. If we could go this route, would we prefer it? --Chris ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
Yeah, I think improving the script would be great. I'm not actually an expert on how it works internally, but I think David Levin is. It's easy for the bot to pass a flag to the script if that would be helpful. In general, I think we should give it a try and iterate to remove the biggest pain points. Thanks! Adam On Sun, Nov 29, 2009 at 5:00 PM, Chris Jerdonek chris.jerdo...@gmail.com wrote: On Sun, Nov 29, 2009 at 9:35 AM, Adam Barth aba...@webkit.org wrote: On Sun, Nov 29, 2009 at 9:26 AM, Chris Jerdonek chris.jerdo...@gmail.com wrote: On Sat, Nov 28, 2009, Adam Barth wrote: Hopefully, the script will improve over time, but it will never be perfect. Can you elaborate on this? For example, are you saying there is a basic reason that the script will always have bugs? Without knowing too much about the script, it seems like it wouldn't be too hard to at least make the false negatives go away. Or are you simply saying that the guidelines and script will never fully capture what we mean by correct style? Does this mean you're volunteering to remove all the false positives and false negatives? :) I was hoping to work on the script eventually, which is partly why I asked for elaboration. All that I meant above is that one could potentially disable (for the bot) the style tests that report false violations, or else reduce their confidence score. That way, if the style bot flags a patch, it is guaranteed to be meaningful without looking at the details of the report. This can only be done, though, if the problems with the script are not so basic that they affect most or many of the tests. (The reverse is not as straightforward, though. It does not seem as easy to change the script -- in a useful way -- so that if it reports that a patch has met the guidelines, then the patch really meets the guidelines.) One basic reason the script isn't perfect is that it's doesn't have a full C++ / Objective-C++ parser. If we could go this route, would we prefer it? --Chris ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style-queue entering beta
fwiw, I'm not aware of any bad error reports that people are hitting in practice although as Adam mentioned it is possible for various reasons. If we hit any incorrect warnings with any frequency, we should disable them or fix them. Of course, there are a lots of WebKit style guidelines that it doesn't catch. One basic reason the script isn't perfect is that it's doesn't have a full C++ / Objective-C++ parser. If we could go this route, would we prefer it? It may be a really good thing. It would depend on how complicated the tool/code got. As far as this tool goes, it is derived from an open source version of what Google uses in house and then originally adapted for WebKit by Shinichiro Hamaji. It primarily relies on regex. In the past I wrote a closed source tool that used a parser to do style fixes in lots of ways, and I appreciated the exactness of the results. One nice thing about the current code is that it is pretty accessible to new folks. I don't think that was as true of the tool I wrote on top of the parser which examined and manipulated syntax trees. Dave PS In fact, some checks by various folks in WebKit were made to catch less things in order to avoid false negatives because noisy incorrect tools are just annoying (and imo frequently are justifiably ignored). On Sun, Nov 29, 2009 at 5:55 PM, Adam Barth aba...@webkit.org wrote: Yeah, I think improving the script would be great. I'm not actually an expert on how it works internally, but I think David Levin is. It's easy for the bot to pass a flag to the script if that would be helpful. In general, I think we should give it a try and iterate to remove the biggest pain points. Thanks! Adam On Sun, Nov 29, 2009 at 5:00 PM, Chris Jerdonek chris.jerdo...@gmail.com wrote: On Sun, Nov 29, 2009 at 9:35 AM, Adam Barth aba...@webkit.org wrote: On Sun, Nov 29, 2009 at 9:26 AM, Chris Jerdonek chris.jerdo...@gmail.com wrote: On Sat, Nov 28, 2009, Adam Barth wrote: Hopefully, the script will improve over time, but it will never be perfect. Can you elaborate on this? For example, are you saying there is a basic reason that the script will always have bugs? Without knowing too much about the script, it seems like it wouldn't be too hard to at least make the false negatives go away. Or are you simply saying that the guidelines and script will never fully capture what we mean by correct style? Does this mean you're volunteering to remove all the false positives and false negatives? :) I was hoping to work on the script eventually, which is partly why I asked for elaboration. All that I meant above is that one could potentially disable (for the bot) the style tests that report false violations, or else reduce their confidence score. That way, if the style bot flags a patch, it is guaranteed to be meaningful without looking at the details of the report. This can only be done, though, if the problems with the script are not so basic that they affect most or many of the tests. (The reverse is not as straightforward, though. It does not seem as easy to change the script -- in a useful way -- so that if it reports that a patch has met the guidelines, then the patch really meets the guidelines.) One basic reason the script isn't perfect is that it's doesn't have a full C++ / Objective-C++ parser. If we could go this route, would we prefer it? --Chris ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
