Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 390043

Summary: feedback on "Ask for review" feature
Product: [ECD] Orion Reporter: Susan McCourt <susan>
Component: GitAssignee: Project Inbox <orion.git-inbox>
Status: RESOLVED WONTFIX QA Contact:
Severity: normal    
Priority: P3 CC: maciej.bendkowski, malgorzata.tomczyk, Szymon.Brandys
Version: 0.5   
Target Milestone: ---   
Hardware: PC   
OS: Windows 7   
Whiteboard:

Description Susan McCourt CLA 2012-09-20 13:35:13 EDT
Today I used "ask for review" for the first time.  very cool to link to it from buzilla!  And my first impression of the page was good, it looks very nice.  but then I got tripped up a little bit getting started.  Here is some feedback/suggestions for improvements, etc.

1) "Unfortunately the commit can not be found in your workspace. To see it try one of the following: "

We don't use the "workspace" terminology very much.  Not to mention, the search for commits is in the remote repositories, too, so it's not an accurate term.  Perhaps:
"Unfortunately the commit cannot be found in any of your repositories.  To access the commit, try one of the following:"

2) The twistie in "Attach the remote to one of existing repositories" is a bit distracting for visually parsing the actions I can take.  The sections are nice for showing the different things I might do, but the fact that one of them has a twistie makes it look "different" than the other flat choices.  Perhaps in this case we should just expand the repository section and not make it expand/collapsible.

3) Immediately I "selected" the repository that I wanted to use, by clicking on the link, and that took me to that repository's page.  It didn't occur to me to look all the way to the right to find the "add remote" command.  Perhaps in this case only we should put the icon immediately next to the repository name vs. flush right on page?   (Or maybe if the link for the repo itself did the "add remote" operation, since that is the default action.  That could be confusing, too, so maybe we don't have a link. ) In any case, I think the command for add remote should be close to the list of repos, maybe in a column between repo name and location.

4) When I tried to add a repository, the repository for the contribution was not filled in...it's telling me on that page what repository the commit is in, do I really have to copy/paste it into that popup?  Further complicating things is that the page is showing the github url in two ways:  with the ".git" extension and without it.  So I have to guess which one to use.  Best if it just knows which remote needs to be added.  I don't mind being prompted to confirm it, but having to put it in is surprising.

When all was said and done, I couldn't get the remote added but I will open a separate bug for that error.
Comment 1 Susan McCourt CLA 2012-09-20 14:01:36 EDT
I was wrong about 4)

for some reason my expectation was that either
a) the add remote command would "just do the work" without prompting..or...
b) the popup was going to show me the URL for confirmation.

I think this is because the "add remote" slideout on the repo page prompts for URL first, and then name.  So I just expected the URL to be the first entry again.  Now I realize that the popup was asking me for a name (it clearly says so in the parameter title).  I wonder if we should show the URL too (prefilled in) to prevent this misunderstanding.

Having figured this part out, I typed in a name for the repo and the remote was presumably added successfully, but at that point there was no new feedback on the "ask for review page."   I wasn't sure what to do next, whether this page was supposed to help me through the rest of the workflow.  I clicked on the repo page link so I could check to see that the repo had been added.  The remote was there.

I tried "open commit" at that point and the commit was not found.  So I figured I should fetch the repo.  Then I realized that fetch had failed because the provided repo link:
https://orion.eclipse.org/git/reviewRequest.html#git@github.com:maciej-bendkowski/orion.client.git_6a6cbe8054541deb6bc41fc7adfccccb630a7e49
required ssh authorization rather than read only.  So I'm guessing that the fetch failed on the ask for review page (it didn't prompt me for credentials.)  So I think we have these issues:

5) There was no error feedback or anything telling me what had happened.
6) we probably should revisit the code that creates these "ask for review" links because we want to ensure the read only repo link is generated rather than the ssh that the contributor is using.

To finish reviewing maciej's code, I had to go to his github page, find the read only link, add it manually, fetch the remote, and then open commit.

I think this page has TONS of potential and if we can work through these issues it will be an awesome thing to show off for 1.0 (sorry I haven't tried it before now, this is the first time I got a bugzilla with the review link in it.)
Comment 2 Maciej Bendkowski CLA 2012-09-21 04:23:13 EDT
(In reply to comment #1)
> 6) we probably should revisit the code that creates these "ask for review"
> links because we want to ensure the read only repo link is generated rather
> than the ssh that the contributor is using.

This seems quite odd since multiple contributions I've prepared were reviewed using the ssh links and neither Gosia nor Szymon had problems fetching. Maybe Gosia could shed some light on it.
Comment 3 Susan McCourt CLA 2012-09-21 10:58:57 EDT
(In reply to comment #2)
> (In reply to comment #1)
> > 6) we probably should revisit the code that creates these "ask for review"
> > links because we want to ensure the read only repo link is generated rather
> > than the ssh that the contributor is using.
> 
> This seems quite odd since multiple contributions I've prepared were
> reviewed using the ssh links and neither Gosia nor Szymon had problems
> fetching. Maybe Gosia could shed some light on it.

Gosia mentioned to me while I was having trouble that she had some problems with review links in the past....
Comment 4 John Arthorne CLA 2015-05-05 14:51:50 EDT
Closing as part of a mass clean up of inactive bugs. Please reopen if this problem still occurs or is relevant to you. For more details see:

https://dev.eclipse.org/mhonarc/lists/orion-dev/msg03444.html