On Thu, Oct 29, 2015 at 6:11 AM, Dan Callaghan <dcallagh(a)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 <dcallagh(a)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-check...
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 <dcallagh(a)redhat.com
>
Senior Software Engineer, Products & Technologies Operations
> Red Hat, Inc.
> _______________________________________________
> Beaker-devel mailing list
> Beaker-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/beaker-devel