Hello folks, On Fri, Sep 27, 2019 at 1:59 PM Amit Langote <amitlangot...@gmail.com> wrote: > > On Fri, Sep 27, 2019 at 12:52 PM Youki Shiraishi <shirai...@computer.org> > wrote: > > On Fri, Sep 27, 2019 at 12:10 AM Amit Langote <amitlangot...@gmail.com> > > wrote: > > > On Thu, Sep 26, 2019 at 6:32 PM Youki Shiraishi <shirai...@computer.org> > > > wrote: > > > > On Thu, Sep 26, 2019 at 5:38 PM Amit Langote <amitlangot...@gmail.com> > > > > wrote: > > > > > + /* Bootstrap mode for initdb */ > > > > > if (argc > 1 && strcmp(argv[1], "--boot") == 0) > > > > > AuxiliaryProcessMain(argc, argv); /* does not return */ > > > > > else if (argc > 1 && strcmp(argv[1], "--describe-config") == 0) > > > > > > > > > > How about expanding that comment just a little bit, say: > > > > > > > > > > /* > > > > > * Bootstrapping is handled by AuxiliaryProcessMain() for historic > > > > > * reasons. > > > > > */ > > > > > > Do you any thoughts on this suggestion? > > > > Sorry, I missed your suggestion. > > The purpose of a comment here is to direct hackers to initdb.c because > > the --boot option is used only by initdb. > > initdb.c describes why it uses the --boot option (i.e., historical > > reason), so I think it should not be described in main.c. > > Sorry, I didn't really mean to take out the "for initdb" part. So, I > should've suggested this > > /* > * Bootstrap mode for initdb. Bootstrapping is handled by > * AuxiliaryProcessMain() for historical reasons. > */ > > IMO, it would be good for this comment to say why > AuxiliaryProcessMain() is invoked here instead of, say, > PostgresMain(). "for historical reasons" may not be enough but maybe > it is.
I totally agree with that. It might also help to tell that AuxiliaryProcessMain() is special entry point compared to others. According to the discussion so far, I revised my patch as attached. I believe the patch might help beginners. Many thanks, -- Youki Shiraishi NTT Software Innovation Center Phone: +81-(0)3-5860-5115 Email: shirai...@computer.org
v3_add_comment_for_bootstrap_mode.patch
Description: Binary data
