hawq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Guo <paul...@gmail.com>
Subject Re: Warnings and indent issues in HAWQ code.
Date Thu, 01 Dec 2016 06:17:37 GMT
I'm proposing the Macros below at first to help eliminate some "unused"
variable/argument warnings.

Typical cases:

1) Variable is only used with some configurations, etc. val for
Assert(val). Then you could add the code below
    to eliminate the warning when assert is not enabled.


2) For argument that is explicitly unused but might be kept for
compatibility, you could use UNUSED_ARG().

[pguo@host67:/data2/github/incubator-hawq-a/src/include]$ git diff
diff --git a/src/include/postgres.h b/src/include/postgres.h
index 1138f20..9391d6b 100644
--- a/src/include/postgres.h
+++ b/src/include/postgres.h
@@ -513,6 +513,18 @@ extern void gp_set_thread_sigmasks(void);

 extern void OnMoveOutCGroupForQE(void);

+#define POSSIBLE_UNUSED_VAR(x) ((void)x)
+#define POSSIBLE_UNUSED_ARG(x) ((void)x)
+#ifndef UNUSED_ARG
+#define UNUSED_ARG(x)          ((void)x)
 #ifdef __cplusplus
 }   /* extern "C" */

2016-12-01 10:26 GMT+08:00 Yandong Yao <yyao@pivotal.io>:

> Great, +dev@hawq.incubator.apache.org
> On Thu, Dec 1, 2016 at 9:54 AM, Paul Guo <pguo@pivotal.io> wrote:
>> Yes, strongly agree.
>> By the way, We should keep updating your story progress on Apache JIRA if
>> your story is not confidential.
>> We should try to avoid closing an Apache JIRA with just a simple
>> introduction, i.e. without analysis, without progress, without root cause.
>> On Thu, Dec 1, 2016 at 9:13 AM, Ruilong Huo <rhuo@pivotal.io> wrote:
>>> Strong +1 to move discussions about features, improvements, usages, etc
>>> in dev@hawq.incubator.apache.org and user@hawq.incubator.apache.org
>>> respectively! This improve interactions between us and developers and user,
>>> which in turn helps to grow hawq community in apache open source society.
>>> ‚Äč
>>> Best regards,
>>> Ruilong Huo
>>> On Thu, Dec 1, 2016 at 8:59 AM, Yandong Yao <yyao@pivotal.io> wrote:
>>>> Thanks Paul for initiating this discussion, +1 for -Werror and using
>>>> pgindent.
>>>> Besides, could we move such discussion to dev@hawq.incubator.apache.org,
>>>> it is perfect topic to discuss in open source community!
>>>> On Wed, Nov 30, 2016 at 5:58 PM, Hong Wu <hwu@pivotal.io> wrote:
>>>>> If we want to do this, I recommend to write the coding style inside a
>>>>> hawq.vim and a hawq.emacs file for developers to use. I agree with you
>>>>> to fix compile warnings and coding styles including indents, we do this
>>>>> together with some bug fix which could minimize our effort.
>>>>> Hong
>>>>> On Wed, Nov 30, 2016 at 5:46 PM, Paul Guo <pguo@pivotal.io> wrote:
>>>>>> Sorry. I meant -Werror (Which means "warning as error").
>>>>>> I did not tried pgindent. Anyone knows this whether works for us.
>>>>>> the way, even this works I suspect there will be tons of warnings
also then
>>>>>> we have to leave this to be fixed along with other fixes.
>>>>>> Besides, pg have a coding style doc but that is very simple. I saw
>>>>>> least our naming conventions are not uniform. This will be an more
>>>>>> issue.
>>>>>> On Wed, Nov 30, 2016 at 5:29 PM, Hong Wu <hwu@pivotal.io> wrote:
>>>>>>> There is no harm to fix compile warnings. Just do it, paul!
>>>>>>> In "enforce -Werror (if gcc) in hawq" thread of dev mailing list,
>>>>>>> we talked about opening "-Werror" but there were some concerns
>>>>>>> the portability issue. I think it's better to fix compile warnings
but do
>>>>>>> not open this option.
>>>>>>> For indent and code style, HAWQ contains a tool for check that
>>>>>>> "src/tools/pgindent" which is forked from original Postgres.
I think we
>>>>>>> should run the tool to check existing code and do some indent
>>>>>>> Meanwhile, we should also use this tool to check our c-coding
indent before
>>>>>>> sending pull requests and reviewer should pay attention to indent
>>>>>>> Best,
>>>>>>> Hong
>>>>>>> On Wed, Nov 30, 2016 at 5:00 PM, Paul Guo <pguo@pivotal.io>
>>>>>>>> Hi team,
>>>>>>>> Long ago, I proposed to add "-Wall" (warning as error) option
>>>>>>>> the community and later found that is a big effort since
warnings are too
>>>>>>>> many, but I think the direction is right. I think we should
try to fix
>>>>>>>> those warnings when you are modifying related source file,
besides, I found
>>>>>>>> indent issue is a bit severe, so if you encountered this
during bug fixes
>>>>>>>> please fix them. All of these will improve the confidence
of users and
>>>>>>>> customers.
>>>>>>>> Thanks.
>>>> --
>>>> Best Regards,
>>>> Yandong
> --
> Best Regards,
> Yandong

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message