[webkit-reviews] review denied: [Bug 33819] Style in WebCore/bridge/jni/jsc/JavaClassJSC needs fixing : [Attachment 46871] Patch 1 for Bug 33819

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 19 09:09:07 PST 2010


Darin Adler <darin at apple.com> has denied Steve Block <steveblock at google.com>'s
request for review:
Bug 33819: Style in WebCore/bridge/jni/jsc/JavaClassJSC needs fixing
https://bugs.webkit.org/show_bug.cgi?id=33819

Attachment 46871: Patch 1 for Bug 33819
https://bugs.webkit.org/attachment.cgi?id=46871&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> Index: WebCore/bridge/jni/jsc/JavaClassJSC.cpp
> ===================================================================
> --- WebCore/bridge/jni/jsc/JavaClassJSC.cpp	(revision 53443)
> +++ WebCore/bridge/jni/jsc/JavaClassJSC.cpp	(working copy)
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2003 Apple Computer, Inc.  All rights reserved.
> + * Copyright (C) 2010 Apple Computer, Inc.  All rights reserved.

Should be "Copyright (C) 2003, 2010 Apple Inc. All rights reserved." and should
also list any other years the code here had significant changes that were
published (checked in to the open source repository). Would take a little
research to determine that.

> +    free((void*)m_name);

Since you're fixing style, this should be const_cast<char*>(m_name) instead of
(void*)m_name.

> - * Copyright (C) 2003 Apple Computer, Inc.  All rights reserved.
> + * Copyright (C) 2010 Apple Computer, Inc.  All rights reserved.

Same comment as above. Please don't erase earlier publication years.

> +    JavaClass(jobject anInstance);

The argument name here does not make it any clearer what the argument is. If we
can't do any better, then we should omit the argument name.

> +    virtual MethodList methodsNamed(const Identifier&, Instance* instance)
const;
> +    virtual Field* fieldNamed(const Identifier&, Instance* instance) const;

The argument name "instance" should be omitted here.

review- because the copyright date should not be changed in this manner


More information about the webkit-reviews mailing list