On Thu, Oct 29, 2015 at 6:11 AM, Dan Callaghan <dcall...@redhat.com> wrote:
> Thanks for the feedback Jeff.
>
> Excerpts from Jeffrey Burke's message of 2015-10-26 07:54 -04:00:
>> On Sun, Oct 25, 2015 at 10:55 PM, Dan Callaghan <dcall...@redhat.com> wrote:
>> > * The original design proposal for the job page improvements mentioned
>> > allowing ack/nak/comment on recipes, tasks, and results. However it's
>> > not clear what the meaning or behaviour of a nak at one of these lower
>> > levels should be, and also unclear if nack'ing individual results is
>> > even useful or desirable, so we will defer this. For now, ack/nak will
>> > only apply to recipe sets as it does now.
>> >
>> Is the original proposal online? All I remember is the BZ.
>> Bug 596410 - [Beaker] RFE: Ability to hide recipes from the matrix view.
>> https://bugzilla.redhat.com/show_bug.cgi?id=596410
>
> When I said "original design proposal" I was talking about the one we
> wrote for the job page improvements earlier this year, where we had
> some quite ambitions plans for expanding ack/nak/comments. We decided
> not to tackle those for now.
>
> As for the original design of the ack/nak system... Yes I think that BZ
> is all we have. It was implemented a long time ago, before we were
> writing up our designs.
>
>> > * The distinction between "needs review" and "ack" does not seem useful
>> > so it will be removed. All recipe sets will effectively by "acked" by
>> > default. (Recipe sets with a Pass result are already acked by default
>> > right now.)
>> >
>> From what I remembered there were two reasons. The reason we wanted
>> both was if multiple people are reviewing the same job. Or if you were pulled
>> away during your review you could come back and pick up where you left off.
>> You could tell which ones were reviewed and which ones still needed to be
>> reviewed.
>
> Hmm interesting. We hadn't thought of either of those scenarios.
>
> As for the second point, keeping track of what you have reviewed...
> Gerrit has quite a nice simple system for that, which we could take some
> inspiration from. Each file in the patch has a little checkbox
> "Reviewed" which is unticked by default. It's ticked automatically when
> you first view each file. And you can toggle them on or off explicitly
> as well, to mark where you are up to in the review. Or you can just
> ignore them.
>
> We could do something similar. The job page could have a checkbox for
> each recipe. We would record each user's "reviewed" status for each
> recipe. When you open the recipe page we mark it as reviewed
> automatically, or you can tick or untick the checkbox explicitly.
> Something like this:
>
> https://fedorapeople.org/~dcallagh/screenshot-new-job-page-reviewed-checkboxes.png
>
> Then waiving and commenting would still be a separate operation. The
> "reviewed" check boxes might be more generally useful even when not
> waiving results too.
>
I like it. The mock up looks good. The only thing that people may want is the
ability to "Check All"
>
> However it doesn't solve the first thing you mentioned, where multiple
> people are reviewing a single job. Does that happen often? I feel like
> it's maybe a situation that we can't really handle well anyway.
>
We have been able to split the review out logically by person. So we very
rarely if ever have multiple people reviewing the same job at the same time.
So I am ok if we drop this requirement
>
Thanks,
Jeff
> --
> Dan Callaghan <dcall...@redhat.com>
> Senior Software Engineer, Products & Technologies Operations
> Red Hat, Inc.
>
> _______________________________________________
> Beaker-devel mailing list
> Beaker-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/beaker-devel
>
_______________________________________________
Beaker-devel mailing list
Beaker-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/beaker-devel
