[webkit-reviews] review denied: [Bug 40352] [Qt] instance objects created for QObjects are somtimes GC'd : [Attachment 58554] Proposed patch with proper coding style

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 15 05:33:57 PDT 2010


Jędrzej Nowacki <jedrzej.nowacki at nokia.com> has denied Sam Magnuson
<smagnuson at netflix.com>'s request for review:
Bug 40352: [Qt] instance objects created for QObjects are somtimes GC'd
https://bugs.webkit.org/show_bug.cgi?id=40352

Attachment 58554: Proposed patch with proper coding style
https://bugs.webkit.org/attachment.cgi?id=58554&action=review

------- Additional Comments from Jędrzej Nowacki <jedrzej.nowacki at nokia.com>
The patch doesn't apply cq-
There are a few unrelated changes and coding style issues (r-)
there are no autotest.

You can use check-webkit-style script to check some coding style issues.

> +	   No new tests. (OOPS!)
Please provide autotest.

>  #include "ArgList.h"
> +#include "Error.h"
>  #include "JSDOMBinding.h"
>  #include "JSGlobalObject.h"
>  #include "JSLock.h"
> +#include "ObjectPrototype.h"
> +#include "PropertyNameArray.h"
>  #include "qt_class.h"
>  #include "qt_runtime.h"
> -#include "PropertyNameArray.h"
>  #include "runtime_object.h"
> -#include "ObjectPrototype.h"
> -#include "Error.h"

> -    foreach(QtInstance* instance, cachedInstances.values(o))
> +    foreach (QtInstance* instance, cachedInstances.values(o))
These changes are unrelated (there are more). Please cleanup the patch.

> +#if 1
> +#endif
This should be avoided.

> +	   } else if ( m_object ) {
A coding style issue.


More information about the webkit-reviews mailing list