A ticket would be nice on this since it is a user facing feature. Is there anything in the README for the docs that should highlight this?
--joel On Fri, Feb 11, 2022 at 8:18 PM Gedare Bloom <ged...@rtems.org> wrote: > > 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 <shashvatjain2...@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, <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 _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
