Feedback from my devs on The Pull Request functionality

swhelan's Avatar

swhelan

04 Nov, 2013 07:52 PM

I asked all my devs for feedback on how Pull Request workflows were working for them. This is a collation of the replies I got:

Main wants

*Pull request that updates with new commits (as per GitHub pull request system). We very frequently have to decline->re-create PR’s for a minor code review changes, is much nicer if it reflects most recent commits.

*Allow a pull request to be regenerated easily after it’s been rejected to include the extra commits since the rejected pull request was generated. This has to be done manually currently and I usually end up keeping the name and description and branches of the new pull request the same as the rejected pull request.

*Auto merge for non-conflicting PR (as per GitHub)

*Make it easier to Close a pull request (have to currently click the vote link next to a comment, vote, tick the Close checkbox and save a comment). Maybe have a close button by itself?

*When doing a pull request, part of our process is to click the "Detailed compare view" button down the bottom - when you do this it opens up in the same window and there is no way of going back without losing all the information you've put in (title/message/reviewers/branches/etc). Should open in a new window or another solution to remember that data and allow you to go back.

*Default reviewers – 99% of the time I am creating a PR from myFork -> upstream with the same reviewers every time.

Other nice to haves

*A trial/preview auto merge, where code is merged, built and unit tested in a trial branch/bookmark (whatever). This is useful for the case where you are largely sure it’s fine but want to make sure it still builds before actually merging in

*Approving/Rejecting should be more apparent as they are major operation, clicking “Vote for pull request” is not where people first look

*Being able to comment on a file as a whole, if a file while was erroneously included, you have to comment on the first line…

  1. 1 Posted by peter.rebholz on 04 Mar, 2014 10:33 PM

    peter.rebholz's Avatar

    I think these are nice suggestions and would solve a lot of the issues that we have with using pull requests for code reviews (we're currently just reviewing in the main repository after changes are pushed). From what I can remember, the last remaining thing that would help us is having the comments get copied or referenced by the main repository as well. The issue that we see is that you can't tell at a glance which commits have been reviewed or easily look at the the review comments for a given commit. Further, based on the database schema, the comments are tied to the fork repository so if, after merging my changes I want to delete my fork I can't do that without loosing those details.

  2. Support Staff 2 Posted by Marcin Kuzminsk... on 05 Mar, 2014 08:27 AM

    Marcin Kuzminski's Avatar

    Hi,

    we're planning a major overhaul of the pull-request/codereview during the next few months. Thanks for all the feedback, those are some really nice ideas !

    We'll be working hard now to improve what we currently have and implement some nice automation APIs and improve current workflows for both code-review and pull-requests.

  3. 3 Posted by peter.rebholz on 05 Mar, 2014 02:36 PM

    peter.rebholz's Avatar

    Marcin,

    Glad to hear that you'll be working on the pull-request/code review functionality. Are you planning posting some sort of documentation about the changes that you're planning on making prior to actually making the changes? It might be nice to get community feedback before implementing the changes.

    I'm more than happy to review/provide feedback, explain our current workflow, etc.

  4. 4 Posted by RhodeCode Admin on 06 Mar, 2014 01:43 PM

    RhodeCode Admin's Avatar

    Hi Peter,

    user input and real-life experience are extremely valuable for our product development and today we are already working closely with some of our largest customers on important features and improvements.

    We are planning to extend this audience to all beta testers and paying customers and are currently discussing the best and most efficient way to implement that.

    Best regards,

    Sebastian

  5. Support Staff 5 Posted by Marcin Kuzminsk... on 15 Jul, 2015 08:27 AM

    Marcin Kuzminski's Avatar

    Hi Peter,

    Sorry for the late response.
    In RhodeCode 3.4.0 released yesterday you can edit PR, as well as define default reviewers. We have addressed some smaller requests from your list, and there is more to come (like a special shadow repos exposing the state after Merge)

    This is a new functionality and we have it as added extensions. I'd create a special package for you to extract and copy over to where your .ini file is placed (most probably ~/.rccontrol/enterprise-1/ directory)

    Please extract this package, to a directory called rcextensions.

    After extraction the location of entry file should be following:
    ~/.rccontrol/enterprise-1/rcextensions/__init__.py

    Then the only things you have to do is for each project you want default reviewers is to set two extra fields, called default_reviewers, and default_reviewers_random_pick.

    I'd attached screenshots displaying how the fields and values should be entered. the random pick can be 0 then all reviewers specified in the list would be added to the pull request.

  6. Marcin Kuzminski closed this discussion on 15 Jul, 2015 08:27 AM.

Comments are currently closed for this discussion. You can start a new one.

Keyboard shortcuts

Generic

? Show this help
ESC Blurs the current field

Comment Form

r Focus the comment reply box
^ + ↩ Submit the comment

You can use Command ⌘ instead of Control ^ on Mac