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 <[email protected]>: > 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 <[email protected]>: > > > Great, [email protected] > > > > On Thu, Dec 1, 2016 at 9:54 AM, Paul Guo <[email protected]> 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 <[email protected]> wrote: > >> > >>> Strong +1 to move discussions about features, improvements, usages, etc > >>> in [email protected] and [email protected] > >>> 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 <[email protected]> wrote: > >>> > >>>> Thanks Paul for initiating this discussion, +1 for -Werror and using > >>>> pgindent. > >>>> > >>>> Besides, could we move such discussion to > [email protected], > >>>> it is perfect topic to discuss in open source community! > >>>> > >>>> On Wed, Nov 30, 2016 at 5:58 PM, Hong Wu <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 > > >
