← Back to team overview

ubuntu-appstore-developers team mailing list archive

Re: Tracking overrides in the review scripts

 

Hello,

thanks for bringing this up again, as it reminded me of some changes done
lately that I wanted to share.

The store has support for automatic approve/reject already (though it's
only enabled on staging until we finish QA), including manual overrides.
The way this works is that instead of relying on a text flag
(MANUL_REVIEW), from now on, any check that wants to flag itself as
requiring manual review needs to return a boolean key in it's json result
data. For example

{"security_template_valid (payui.json)": {"text": "'unconfined' not
allowed", "manual_review": True}}

The current policy for automatic approve/reject is

1. If there are errors in the automated review results and none of them is
marked as requiring manual review, the app is automatically rejected
2. If there are errors in the automated review results and some of them are
marked as requiring manual review, the app is left in the review queue
waiting for a reviewer to pick it up and start a review (just like it is
now)
3. If there are no errors and no warnings in the automated review results,
the app is automatically approved.

Additionally, one of the upcoming changes is that the automated reviews
results will include the revno of the click-reviewers-tools branch used to
run the checks. These results will be only visible by reviewers (while the
rest of the review results are also visible to the app developer). If for
some reason it's desired that the results of a check from
click-reviewers-tools should be visible only by a reviewer and not by a
developer, this can be indicated by a 'reviewer' boolean key in the check
result, like

{"click-reviewers-tools revision": {"text": "r239", "reviewer": True}}

While all of this allows us to control which checks require manual review
and the visibility of each check in the store's review page, it still
requires changes to the click-reviewers-tools branch to mark each check
appropriately. These changes don't attempt to solve the "let's have a
single unified place to determine whether a check should require manual
review or not" problem.

Hope this helps,
Ricardo

On Wed, Sep 17, 2014 at 4:33 AM, Daniel Holbach <daniel.holbach@xxxxxxxxxx>
wrote:

> Hello,
>
> On 29.08.2014 16:11, Daniel Holbach wrote:
> > it'd make Alan and my lives easier (until there's automatic
> > reviews/rejections/etc.), if we could somehow track current overrides.
> > Currently there's mainly the core apps and things such as the PayUI
> > which override some of the errors and warnings which would normally
> > block apps from going through.
> >
> > Is there a way you would like something like this to be implemented?
> >
> > My idea would have been to change the output of click-review to simply
> show:
> >
> >  - Known overrides
> >  - new errors/warnings
> >  - general info messages
> >
> > If there are no new errors or warnings, it would simply say "pass" and
> > let us approve the app.
> >
> > To track known overrides, we could maybe just store a piece of json like
> > this somewhere:
> >
> > {
> >   "com.canonical.payui":
> >   {
> >     "security_template_valid (payui.json)":
> >       "(MANUAL REVIEW) 'unconfined' not allowed",
> >     "lint_hooks_redflag_payui":
> >       "(MANUAL REVIEW) 'pay-ui' not allowed"
> >   }
> > }
>
> Any thoughts?
>
> Have a great day,
>  Daniel
>
> --
> Mailing list: https://launchpad.net/~ubuntu-appstore-developers
> Post to     : ubuntu-appstore-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~ubuntu-appstore-developers
> More help   : https://help.launchpad.net/ListHelp
>

References