[PATCH rtems-docs] Add option --build-manuals to build multiple specific manuals.
Gedare Bloom
gedare at rtems.org
Sat Feb 12 02:17:52 UTC 2022
That looks better. My other comments still need to be addressed. You
should also make sure the patch applies cleanly to a fresh repo. I
think I saw some whitespace problems.
It will be easier to review if you can figure out how to get
git-send-email to work.
On Fri, Feb 11, 2022 at 10:43 AM Shashvat <shashvatjain2002 at gmail.com> wrote:
>
> Here is the patch, I ran yapf on it and it didn't introduce any changes. Please take a
> look and let me know what you think.
>
> On Fri, 11 Feb 2022, 10:31 pm Gedare Bloom, <gedare at rtems.org> wrote:
>>
>> On Fri, Feb 11, 2022 at 9:38 AM Shashvat <shashvatjain2002 at gmail.com> wrote:
>> >>
>> >>
>> >
>> > Hi Gedare!!
>> >
>> >> Is there a ticket associated with this, or any feature request? Or
>> >> just something you thought of doing?
>> >
>> >
>> > I am sorry I should have mentioned the motive behind the option.
>> > I was planning to work on ticket #3333 which works on a specific posix-users manual afaik. I wanted waf to build only this particular manual so asked Chris on discord if it is possible and he told me how it was broken, and that it would be good to add an option that enables this.
>> >>
>> >>
>> >> I don't think an extra blank line is needed here.
>> >
>> >
>> > This was my fault.
>> >>
>> >> >
>> >> > $ ./waf
>> >> >
>> >> > @@ -448,8 +450,10 @@ verbose level:
>> >> > $ ./waf configure --sphinx-options "-V -V"
>> >> > $ ./waf clean build
>> >> >
>> >> > -You can enter a manual's directory and run the same configure command and
>> >> > build
>> >> > -just that manual.
>> >> > +If you wish to build only some specific manuals,
>> >> > +use the '--build-manuals=<manual-name-1>,<manual-name-2>' option with
>> >> > +configure to build only those specific manuals.
>> >> > +
>> >> >
>> >> > Documentation Standard
>> >> > ----------------------
>> >> > diff --git a/common/waf.py b/common/waf.py
>> >> > index fa9aecb..e6ae059 100644
>> >> > --- a/common/waf.py
>> >> > +++ b/common/waf.py
>> >> > @@ -240,6 +240,11 @@ def cmd_configure(ctx):
>> >> > check_sphinx_extension(ctx, 'sphinxcontrib.bibtex')
>> >> >
>> >> > #
>> >> > + # Build specific manuals.
>> >> This spacing looks wrong.
>> >>
>> >> > + #
>> >> > + if ctx.options.build_manuals!="":
>> >> Follow the coding style of the surrounding text. For Python code, we
>> >> generally follow
>> >> https://docs.rtems.org/branches/master/eng/python-devel.html
>> >
>> >
>> > Thanks, I will take a look.
>> >>
>> >>
>> >> > + ctx.env.MANUALS = ctx.options.build_manuals.split(',')
>> >> Probably want a blank line here. None of the other options take
>> >> multiple values. I wonder if there is any value to having a multiple
>> >> option here, versus building just one manual selectively? You could
>> >> still have an 'all' option as the default. That will reduce the
>> >> complexity of the command line argument processing.
>> >
>> >
>> >>
>> >> I guess by default this will not build any manuals? The principle of
>> >> least surprise suggests that by default the behavior should be what it
>> >> used to be if you omit the argument, so build everything. otherwise,
>> >> you break existing workflows and scripts.
>> >
>> >
>> > This does has the action of building all manuals by default.
>> >>
>> >>
>> >> This is not good python, no indent after 'if', so there's nothing in
>> >> the conditional code block, you just always set building to
>> >> ctx.env.MANUALS.
>> >>
>> >> > + print("Building the following manuals:-")
>> >> > + for manual in building:
>> >> > + print(manual)
>> >> missing indent here too. But the print statements seem to be
>> >> inconsistent with other printed output for this code. You generally
>> >> want to keep that consistent.
>> >
>> >
>> > This is my first time submitting a patch by mail and looks like I messed something up up while copying the diff, should have checked by applying the diff before submitting. Looks like the indents are missing :)
>> >
>> If you don't have git-send-email working, then use git-format-patch
>> and just email the patch instead of trying to copy-paste into your
>> emailer
>>
>> >
>> > Thanks
>> > Shashvat
More information about the devel
mailing list