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, <ged...@rtems.org> wrote: > On Fri, Feb 11, 2022 at 9:38 AM Shashvat <shashvatjain2...@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 > _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
