The user interface improvements for the job page and recipe page [1] have been in planning/discussion for quite a while now, but recently the team has been making good progress on prototyping the new interfaces with a view towards implementing them in Beaker 22.0.
One issue we ran up against while prototyping the job page was how to handle the ack/nak/comment functionality for recipe sets. In the original design document we essentially hand-waved past this but when it came down to actually implementing something we realised the current UI with a tri-state selector plus comment modal does not fit in with the new design, and it offers a very subpar user experience as well.
After some discussion the Beaker development team came up with the following plan for tackling the ack/nak/comment issues:
* The comment functionality will be separated out from ack/nak and fully fleshed out, supporting multiple comments from different users and recording the commenting user and timestamp. It will also be expanded to support commenting on tasks and results, which was one of the key requested items for Beaker 22.0.
* 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.
* The terms "ack" and "nak"/"nack" have no precedent in other testing systems and their meaning is unclear to new users. The "nak" setting will be renamed to "waived" in line with its intended meaning. However the existing response="nak" attribute in the job results XML will be preserved for compatibility.
* 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.)
In particular this means the operation of changing a failed recipe set from "needs review" to "ack" will no longer be possible. The only operation will be "waive" which is equivalent to nack'ing a recipe set, or "un-waive" to reverse it.
If you see any issues with the above, or if you think it might break your workflows, please hit reply.
[1] https://beaker-project.org/dev/proposals/job-page-improvements.html
On Sun, Oct 25, 2015 at 10:55 PM, Dan Callaghan dcallagh@redhat.com wrote:
The user interface improvements for the job page and recipe page [1] have been in planning/discussion for quite a while now, but recently the team has been making good progress on prototyping the new interfaces with a view towards implementing them in Beaker 22.0.
One issue we ran up against while prototyping the job page was how to handle the ack/nak/comment functionality for recipe sets. In the original design document we essentially hand-waved past this but when it came down to actually implementing something we realised the current UI with a tri-state selector plus comment modal does not fit in with the new design, and it offers a very subpar user experience as well.
After some discussion the Beaker development team came up with the following plan for tackling the ack/nak/comment issues:
- The comment functionality will be separated out from ack/nak and fully fleshed out, supporting multiple comments from different users and recording the commenting user and timestamp. It will also be expanded to support commenting on tasks and results, which was one of the key requested items for Beaker 22.0.
That sounds like good.
- 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
The terms "ack" and "nak"/"nack" have no precedent in other testing systems and their meaning is unclear to new users. The "nak" setting will be renamed to "waived" in line with its intended meaning. However the existing response="nak" attribute in the job results XML will be preserved for compatibility.
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.
In particular this means the operation of changing a failed recipe set from "needs review" to "ack" will no longer be possible. The only operation will be "waive" which is equivalent to nack'ing a recipe set, or "un-waive" to reverse it.
If you see any issues with the above, or if you think it might break your workflows, please hit reply.
I don't think the changes will break anything. People may have to adjust the way they review jobs.
[1] https://beaker-project.org/dev/proposals/job-page-improvements.html
-- Dan Callaghan dcallagh@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
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@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-checkbox...
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.
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.
On Thu, Oct 29, 2015 at 6:11 AM, Dan Callaghan dcallagh@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@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-checkbox...
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@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
Hi all,
comment following:
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-checkbox...
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"
This sounds like there is need for bulk actions. In such case I quite do not like this mock up. I would rather see those tick boxes in front of each of the job (row) to be used for possible bulk actions (e.g. "Mark checked", "Delete", "Cancel" and so on) and use another way to present that particular job has been reviewed - another column with some icon (I suppose column should have some title)? Or highlight whole row with some color? Or just part of it? Whyt?
Cheers,
Jan
----- Original Message -----
From: "Jan Stourac" jstourac@redhat.com To: "Beaker development" beaker-devel@lists.fedorahosted.org Sent: Thursday, October 29, 2015 1:17:19 PM Subject: Re: [Beaker-devel] Proposed changes to recipe set ack/nak/comments
Hi all,
comment following:
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-checkbox...
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"
This sounds like there is need for bulk actions. In such case I quite do not like this mock up. I would rather see those tick boxes in front of each of the job (row) to be used for possible bulk actions (e.g. "Mark checked", "Delete", "Cancel" and so on) and use another way to present that particular job has been reviewed - another column with some icon (I suppose column should have some title)? Or highlight whole row with some color? Or just part of it? Whyt?
Oh, ok - now I see that given mock up page shows view of recipes in job. I suppose that you can ignore my comment now :) Sorry for confusion here.
Jan
Excerpts from Jan Stourac's message of 2015-10-29 08:25 -04:00:
This sounds like there is need for bulk actions. In such case I quite do not like this mock up. I would rather see those tick boxes in front of each of the job (row) to be used for possible bulk actions (e.g. "Mark checked", "Delete", "Cancel" and so on) and use another way to present that particular job has been reviewed - another column with some icon (I suppose column should have some title)? Or highlight whole row with some color? Or just part of it? Whyt?
Oh, ok - now I see that given mock up page shows view of recipes in job. I suppose that you can ignore my comment now :) Sorry for confusion here.
No it's a fair point. I was worried about the same thing -- we do plan on implementing the bulk actions idea eventually, on the jobs grid and recipes grid. This is the job page, so it won't have bulk actions, but it is potentially quite confusing that we have a similar checkbox layout serving a totally different purpose on this page.
But in the end we decided to just go with it, because this layout for "reviewed" checkboxes seems fairly natural and it works well in Gerrit and we couldn't come up with any better alternative layouts for it.
beaker-devel@lists.fedorahosted.org