hawq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hong Wu <xunzhang...@gmail.com>
Subject Re: Warnings and indent issues in HAWQ code.
Date Thu, 01 Dec 2016 07:37:17 GMT
Thanks Paul to point it out. I will start fixing these warnings as possible
as I can!

Hong

2016-12-01 14:17 GMT+08:00 Paul Guo <paulguo@gmail.com>:

> 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.
>
>    POSSIBLE_UNUSED_VAR(val);
>
> 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);
>
> +#ifndef POSSIBLE_UNUSED_VAR
> +#define POSSIBLE_UNUSED_VAR(x) ((void)x)
> +#endif
> +
> +#ifndef POSSIBLE_UNUSED_ARG
> +#define POSSIBLE_UNUSED_ARG(x) ((void)x)
> +#endif
> +
> +#ifndef UNUSED_ARG
> +#define UNUSED_ARG(x)          ((void)x)
> +#endif
> +
>  #ifdef __cplusplus
>  }   /* extern "C" */
>  #endif
>
> 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 that
> >>>>> 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. By
> >>>>>> 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
> at
> >>>>>> least our naming conventions are not uniform. This will be an
more
> annoying
> >>>>>> 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
> about
> >>>>>>> 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 at
> >>>>>>> "src/tools/pgindent" which is forked from original Postgres.
I
> think we
> >>>>>>> should run the tool to check existing code and do some indent
> fixes.
> >>>>>>> Meanwhile, we should also use this tool to check our c-coding
> indent before
> >>>>>>> sending pull requests and reviewer should pay attention
to indent
> too.
> >>>>>>>
> >>>>>>> Best,
> >>>>>>> Hong
> >>>>>>>
> >>>>>>> On Wed, Nov 30, 2016 at 5:00 PM, Paul Guo <pguo@pivotal.io>
wrote:
> >>>>>>>
> >>>>>>>> Hi team,
> >>>>>>>>
> >>>>>>>> Long ago, I proposed to add "-Wall" (warning as error)
option in
> >>>>>>>> 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
> >
>

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