Code review comment for lp:~kissiel/checkbox/fix-1391774-required-field-in-tp

General comment, don't include the "impl" part in plainbox commit messages.
Push stuff _earlier_ before stuff is done as any re-design changes are cheaper.
Please fix typos in commit messages below.

Please patch plainbox to run the mandatory part if plainbox run -T is used, then you probably won't have to patch checkbox-ng at all.

I want to see a design decision on what happens if a job is selected by both normal and mandatory parts. I think it should trigger a warning from the test plan validator and that this job should be impossible to de-select in checkbox-ng/gui/touch later.

Looking at the get_$newthing_qualifier() it seems like a copy-paste of the other one, perhaps we want a new private method and have all the old methods and the new method call that instead.

Please expand the manual page to include some examples. I think we should include a very useful certification example that says "if you want a test plan that works for certification purpose, please add this snippet "required: ..."

Oh, and the word required is better but I think I still prefer mandatory as it's used similarly in other products. I'd put my $0.90 on mandatory-include instead.

More reviews next week. Thanks.