tlhobbes
New Contributor
Joined 12 years ago
User Profile
User Widgets
Contributions
Re: Do not remove Approval when Poked (both individual Pokes and via Poke Everyone)
I believe poke's UI still needs addressed as a destructive action. I see your point that there is a constructive use-case for poking a reviewer out of approval to get them to re-review (and I overlooked that case in my original post; I agree, that being able to do that should remain a feature in some way); however, there is still the undesirable destructive case of unintentionally/accidentally poking a reviewer out of approval, which is also done via the same single click (of Poke or Poke All). I see now that this is a classic UI design issue, and the general solution is: don't allow destructive actions via a single click, instead present the user with confirmation UI which explains the consequences and allows the user to proceed or cancel the action. Doing so allows the action to still occur as feature, and also greatly helps prevent the action from accidentally occurring. In the case of Poke and Poke All: if there is one or more users that would be poked out of approval, then show a confirmation UI; otherwise, there is no destructive consequence so skip showing confirmation UI. When poking a single user out of Approval, then the confirmation can be as simple as: "Poke will pull <username> out of Approval, proceed?" Ok/Cancel. In the case of poking multiple users at once, where at least one of them would be poked out of approval, then the confirmation UI could be similar to the single-user-poke confirmation (e.g. list all of the users that would be poked out of approval); or perhaps it would show all of the users that would be poked, in two lists: 1) those that would be poked out of approval and 2) everyone else; then the confirmation UI could have 3 choices, such as: Poke All, Poke Non-Approved Only, Cancel. Your suggestion of a new feature, that is somewhere between emailing a user and poking them, may still be useful in addition to the above; but it isn't enough alone to address the destructive case of Poke/Poke All. I believe the most pertinent issues to address in codecollab (other than bugs that break the review process) is to further streamline and optimize the review process by eliminating (where possible) and otherwise preventing (via UI confirmation or options) all cases that would otherwise unnecessarily pull a Reviewer out of Approval; and secondarily similar cases that unnecessarily pull any user out of Waiting to Active. All such annoying cases lead to confusion and lost development time. Poke and Poke All are just one case of this bigger general issue.7.8KViews0likes0CommentsRe: Do not remove Approval when Poked (both individual Pokes and via Poke Everyone)
MrDubya sure but "Email Everyone" won't change the "Waiting" statuses of users to "Active", so your suggested alternative is not a full solution; I concur with you on "changes their status back to active, which is a useful feature to bring someone back into the review" because, for example, that makes the review show near the top of their list of reviews (with a red up arrow instead of near the bottom with a yellow down arrow) and so on - which is better than simply getting only an email notification.7.9KViews0likes0CommentsRe: Show state as "Waiting (was approved)" or similar
Regarding this, it might actually be better if when a Reviewer clicks the overall Approve button, that it logs in the Chat box about just that (who approved with optional date-time stamp). Then overall approval status can be seen within context of the file activity logs that are auto-generated there in the Chat box; in turn that provide great context as to exactly what files changed after a particular reviewer had Approved the review! Granted that could get a bit noisy, taking up vertical lines in the Chat box; so consider simplifying it down to a single line which can list out multiple reviewers/observers who have Approved the review up to that point in the Chat log.12KViews0likes0CommentsRe: Is there a way to stop a poke from causing everyone to re-approve?
I wish SmartBear would address that issue too, so I wrote up a feature request for it here: http://community.smartbear.com/t5/Collaborator-Feature-Requests/Do-not-remove-Approval-when-Poked-both-individual-Pokes-and-via/idi-p/106208 I also wrote up couple other related requests (regarding being thrown out of Approval): http://community.smartbear.com/t5/Collaborator-Feature-Requests/Approve-Until-like-Wait-options/idi-p/106209 http://community.smartbear.com/t5/Collaborator-Feature-Requests/Show-state-as-quot-Waiting-was-approved-quot-or-similar/idi-p/106210 Cheers1.5KViews0likes0CommentsShow state as "Waiting (was approved)" or similar
It's annoying to get thrown out of Approval in several cases; but some cases are valid. Either way, what can be further annoying is not having Code Collaborator indicate any information in the code review webpage on whether or not reviewers had previously been in the Approval state. It would be nice, if the State column, indicated in some way, that users (reviewers/observers/whatever) had previously been in an Approved state but now are not. With so many code reviews going on, it's easy to second-guess yourself on "didn't I approve this already"? And having the tool help keep oneself up-to-date on such matters would be handy. Also, as an Author, it'd be nice to see the same information; as that may help lead the author to know which subset of reviewers really needs to pay more attention to the review (because perhaps the reviewer's that need to re-approve in this case were and are actively and responsively enganged with the review, but some subset of other reviewers are behind needing to get to it still).12KViews8likes5CommentsApprove ... Until (like Wait options)
In general, Code Collaborator has several annoying and time-wasting cases of throwing reviewers out of approval; this feature would help address some of them: Upon clicking "Wait" there are several options to control when you will be pulled out of Waiting. It would super nice if there were similar options added for "Approve". For example: I'd like to click "Approve" and have it present a similar list of options. Givne the current behavior of Approve, it seems that the option selected would be "Activity by Author Occurs". Then I'd like to select a different option, like "File Activity Occurs" so that I can stay in the Approval state, despite newly added comments by the author. This would help prevent annoying cases such as throwing reviewers out of Approval near the end of reviews; say because some other reviewer posted a question to which the author responds (perhaps to educate them on something) but where the new comments are trivial to the actual approval-ness of the review. This should not throw the approved reviewers out of Approval, but currently the authors comment does, and so that just turns into annoying time-waste communication between author and reviewer (via Pokes, email, chats, in-person, etc) to get reviews re-approved. At some in a code review, reviewers are satisfied with the code and approve based on that, so any further comments afterward, without file activity, is most often just unnecessary noise to them.2.8KViews3likes0CommentsDo not remove Approval when Poked (both individual Pokes and via Poke Everyone)
If a Reviewer (or Observer) is in the Approval state, then it is totally unnecessary, and indeed quite annoying and time-wasting, to have that reviewer taken out of Approval upon that reviewer being poked. It would be much better, if Poking a user (via individual Poke or via Poking Everyone) left users in their state of Approval. This would allievate issues such as: 1) Author clicks "Poke Everyone" (perhaps because several reviewers need to look at the review), but the unintended consequence is that it can throw a couple other reviewers out of Approval (thus wasting their time, because nothing else changed up to that point that would have otherwise thrown them out of Approval). Sure, the author could have individually poke only the necessary subset of users, but that less desirable alternative basically leaves the "Poke Everyone" button mostly useless. 2) Similarly, an Author may accidentaly poke a specific user; maybe missed the State column's claim that the user was in Approval and then miss-poked the user. Either way, it's the same downsides as above, but with no alternative via Code Collab. In general, Code Collaborator has several annoying and time-wasting cases of throwing reviewers out of approval; this is one of them; please do fix! Thanks :-)8.3KViews8likes8CommentsGold Stars
Feature suggestion: Add gold stars to reviews. Code Collaborator is great. But it's often a critiquing tool with an overall non-positive feel. Not that it's bad or negative, but it seems to lack some great, frivilous, fun, feedback feature, like giving out gold stars for good things. One can write nice and positive comments in a review. But those must be marked as read by all reviewers in order to finish a review (at least in my experience). So that doesn't really serve the same purpose. Some just want to see a great line of code in a review and award a Gold Star :) It really shouldn't impede the review process in any way. By all means, if you do add something like this, stretch your creativity and make the feature even better!8.4KViews0likes1Comment