b4mboo / git-review Goto Github PK

git review create
/usr/lib/ruby/gems/1.8/gems/octokit-0.6.5/lib/octokit.rb:17:in method_missing': undefined methodtoken=' for Octokit:Module (NoMethodError)
from /usr/lib/ruby/gems/1.8/gems/git-review-0.8.2/bin/../lib/git-review.rb:329:in configure_github_access' from /usr/lib/ruby/gems/1.8/gems/octokit-0.6.5/lib/octokit/configuration.rb:26:inconfigure'
from /usr/lib/ruby/gems/1.8/gems/git-review-0.8.2/bin/../lib/git-review.rb:327:in configure_github_access' from /usr/lib/ruby/gems/1.8/gems/git-review-0.8.2/bin/../lib/git-review.rb:175:ininitialize'
from /usr/lib/ruby/gems/1.8/gems/git-review-0.8.2/bin/git-review:8:in new' from /usr/lib/ruby/gems/1.8/gems/git-review-0.8.2/bin/git-review:8 from /usr/bin/git-review:19:inload'
from /usr/bin/git-review:19

Since branches created with git-review have a common prefix, it should be relatively easy to clean up obsolete ones. We just have to determine when they are no longer needed. An additional command "clean" or "sweep" should delete all local and remote branches whose commits have already been merged.

There should be an option to list all of a request's commits, including their SHAs and commit messages. Maybe change 'git review show x --full' to work like that and move the diff to a separate command 'git review show x --diff'.

When displaying diffs, it would be much better to show the diff against the current master on GitHub and not the local one.

As of time when travis.yml was created ruby-head was 1.9.3, currently there are several errors about new ruby 2.0 specs.

Like:

• defining method on class leads to problems on 2.0 therefore on head

[https://github.com/rspec/rspec-core/pull/873]

private method `define_method' called ...

• workaround introduces in 6bd9f8e with setting private methods to public is not the best idea because exatly RSpec was responsible before on workaround on that to make DSL more powerful. Now it's kind of two-way bug - rspec/ruby
• ruby 2.0

I understand that we definitely need to handle private functional somehow and do :send all the the time not an option neither, but we should discuss how to treat it.

When creating a new request, insert commit messages (that are not yet in master) into body (since this will be displayed inside the mail that is sent out).

When calling git-review without any arguments it shows the help. However, this help states that calling adding the argument --help would show this output, which it doesn't. It actually fails.

Additionally, I couldn't find an option to hide a command from this output. It's not worth it to open a separate issue for this, but if we are working on the help page, we could take a look at whether we can hide the console command, such that it will still work, but not show up in the help.

When creating a new request, create and push to a remote branch if necessary.

Inline comments don't get counted to the overall comment count on git review list. This is misleading to potential reviewers, as the might think noone reviewed that request so far.

Whenever "push" to the remote branch fails and git-review continues anyways it leads to unexpected results. e.g. creating pull requests without the latest changes.

Ever since moving to the latest version of octokit (and therefore GitHub's API v3) the comments in git review show #id --full aren't showing anymore. Instead there is an error message This needs to be updated to work with API v3..

Comment structure has changed and git-review needs to be adjusted. However, since I need this tool to work and octokit 0.6.5 broke, I still released a new version. I promise to release an updated version very quickly.

A command to just write a standardized comment to approve a certain request as being reviewed would make life a lot easier.

When calling git review list I can't see pull requests, which are made from the branch the user is currently on.
In general, one would expect to see all PRs for the repository.

Example:
This PR does not show up if you are on the branch foo/bar: b4mboo/trash#44

Git-review is quite slow for commands like list and show, due to the fact that it needs to get all pull requests from Github every time.

I'm thinking of caching the pull requests locally, and using Github API's if-modified-since before retrieving a entire new list of pull requests.

I'm going to try that to see if it can indeed speed up those commands.

Since all the branches that have been prepared with git-review use the same kind of prefix, it should be quite simple to show a list active feature branches. We're looking for branches with the git-review prefix that have unmerged commits on them, but aren't associated with a (open or closed) pull request.

I imagine the output to look a bit like what we get from git review list. Columns should be date, feature name, branch name. Once issue #47 has been implemented, we could even add a column for the user name to that list. Sort either by date (default) or user name (again after #47).

This might be either a new command (i.e. git review branches) or a switch for an existing one (i.e. git review list --prepared).

Example use case:
Alice creates a feature branch and wants Bob to collaborate. She pushes the branch to remote, such that Bob can check it out. Since the repository they are working on has a lot of branches it is sometimes hard to find the correct branch right away. With this feature Bob could list all relevant branches to choose from.

Support creating requests to other repositories and branches (like the original repo, this has been forked from).

Add new command 'prepare' to create a local branch to work on, which follows the naming conventions, but does not yet push to remote. This should work regardless of existing commits on master. If there are any, they should be moved to the new branch.

Whenever a user merges a request and by that creates an additional merge commit, that commit might get lost when rebasing. Doing that will result git-review not being able to properly close a request anymore.
Workaround: Manually close the request using git review close <ID>.

> sudo gem in git-review
Fetching: addressable-2.2.6.gem (100%)
Fetching: launchy-2.0.5.gem (100%)
Fetching: multipart-post-1.1.5.gem (100%)
Fetching: rack-1.4.1.gem (100%)
Fetching: faraday-0.7.6.gem (100%)
Fetching: faraday_middleware-0.8.4.gem (100%)
Fetching: hashie-1.2.0.gem (100%)
Fetching: multi_json-1.0.4.gem (100%)
Fetching: octokit-1.0.0.gem (100%)
Fetching: yajl-ruby-1.1.0.gem (100%)
Building native extensions.  This could take a while...
Fetching: git-review-1.0.5.gem (100%)
Successfully installed addressable-2.2.6
Successfully installed launchy-2.0.5
Successfully installed multipart-post-1.1.5
Successfully installed rack-1.4.1
Successfully installed faraday-0.7.6
Successfully installed faraday_middleware-0.8.4
Successfully installed hashie-1.2.0
Successfully installed multi_json-1.0.4
Successfully installed octokit-1.0.0
Successfully installed yajl-ruby-1.1.0
Successfully installed git-review-1.0.5
11 gems installed
Installing ri documentation for addressable-2.2.6...
Installing ri documentation for launchy-2.0.5...
Installing ri documentation for multipart-post-1.1.5...
Installing ri documentation for rack-1.4.1...
Installing ri documentation for faraday-0.7.6...
Installing ri documentation for faraday_middleware-0.8.4...
Installing ri documentation for hashie-1.2.0...
Installing ri documentation for multi_json-1.0.4...
Installing ri documentation for octokit-1.0.0...
Installing ri documentation for yajl-ruby-1.1.0...
Installing ri documentation for git-review-1.0.5...
Installing RDoc documentation for addressable-2.2.6...
Installing RDoc documentation for launchy-2.0.5...
Installing RDoc documentation for multipart-post-1.1.5...
Installing RDoc documentation for rack-1.4.1...
Installing RDoc documentation for faraday-0.7.6...
Installing RDoc documentation for faraday_middleware-0.8.4...
Installing RDoc documentation for hashie-1.2.0...
Installing RDoc documentation for multi_json-1.0.4...
Installing RDoc documentation for octokit-1.0.0...
Installing RDoc documentation for yajl-ruby-1.1.0...
Installing RDoc documentation for git-review-1.0.5...
jmason@t3400:~/studio> git review list
/usr/lib/ruby/gems/1.8/gems/faraday_middleware-0.8.4/lib/faraday_middleware/response/parse_json.rb:9: uninitialized constant FaradayMiddleware::ParseJson::JSON (Faraday::Error::ParsingError)
    from /usr/lib/ruby/gems/1.8/gems/faraday_middleware-0.8.4/lib/faraday_middleware/response_middleware.rb:48:in `call'
    from /usr/lib/ruby/gems/1.8/gems/faraday_middleware-0.8.4/lib/faraday_middleware/response_middleware.rb:48:in `parse'
    from /usr/lib/ruby/gems/1.8/gems/faraday_middleware-0.8.4/lib/faraday_middleware/response_middleware.rb:39:in `process_response'
    from /usr/lib/ruby/gems/1.8/gems/faraday_middleware-0.8.4/lib/faraday_middleware/response_middleware.rb:32:in `call'
    from /usr/lib/ruby/gems/1.8/gems/faraday-0.7.6/lib/faraday/response.rb:62:in `on_complete'
    from /usr/lib/ruby/gems/1.8/gems/faraday_middleware-0.8.4/lib/faraday_middleware/response_middleware.rb:30:in `call'
    from /usr/lib/ruby/gems/1.8/gems/faraday-0.7.6/lib/faraday/response.rb:8:in `call'
    from /usr/lib/ruby/gems/1.8/gems/faraday-0.7.6/lib/faraday/request/json.rb:32:in `call'
    from /usr/lib/ruby/gems/1.8/gems/faraday-0.7.6/lib/faraday/connection.rb:210:in `run_request'
    from /usr/lib/ruby/gems/1.8/gems/faraday-0.7.6/lib/faraday/connection.rb:93:in `get'
    from /usr/lib/ruby/gems/1.8/gems/octokit-1.0.0/lib/octokit/request.rb:28:in `send'
    from /usr/lib/ruby/gems/1.8/gems/octokit-1.0.0/lib/octokit/request.rb:28:in `request'
    from /usr/lib/ruby/gems/1.8/gems/octokit-1.0.0/lib/octokit/request.rb:10:in `get'
    from /usr/lib/ruby/gems/1.8/gems/octokit-1.0.0/lib/octokit/client/pulls.rb:24:in `pull_requests'
    from /usr/lib/ruby/gems/1.8/gems/octokit-1.0.0/lib/octokit.rb:18:in `send'
    from /usr/lib/ruby/gems/1.8/gems/octokit-1.0.0/lib/octokit.rb:18:in `method_missing'
    from /usr/lib/ruby/gems/1.8/gems/git-review-1.0.5/bin/../lib/git-review.rb:299:in `update'
    from /usr/lib/ruby/gems/1.8/gems/git-review-1.0.5/bin/../lib/git-review.rb:240:in `initialize'
    from /usr/lib/ruby/gems/1.8/gems/git-review-1.0.5/bin/git-review:8:in `new'
    from /usr/lib/ruby/gems/1.8/gems/git-review-1.0.5/bin/git-review:8
    from /usr/bin/git-review:19:in `load'
    from /usr/bin/git-review:19

Removed octokit, and added octokit 0.6.5 , then things went back to working.

If a user is working on a forked repository and wants to create a PR against upstream, it will fail in case the forked repo's master and the feature branch don't differ. Problem here is that git-review creates a feature branch (unless it already exists) and then compares this feature branch with the local master. If there are no additional commits on the feature branch execution will stop. To fix this, we need to compare with the remote upstream master as target_branch not the local (forked) one.

To reproduce (and to be used as a blueprint for the spec/test):

• fork a repo
• commit some changes
• push to remote
• try to create a pull request against upstream

Bug reporter: @digitaltom

There can only be one pull request from one branch to another at any given time. If that pull request already exists, calling git review create again will fail. Unfortunately, the standard GitHub error is pretty cryptic and people don't understand what went wrong. We need to take care of this, check if the request already exists and output an understandable error message in case it does.

It would be cool if git review uses by default (when not entering anything) the commit
message of the last commit as name.

This is a major bug. Ever since moving to API v3 the merge command will run into a nil error. I'm gonna fix that over the weekend. Promise.

while git-review working as expected. That seems that git as system command do not load any info about ruby version from env

/Users/kalabiyau/.rvm/gems/ruby-1.9.3-p385/gems/git-review-1.1.6/bin/../lib/git-review.rb:11: undefined method `require_relative' for main:Object (NoMethodError)
    from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/rubygems/custom_require.rb:31:in `gem_original_require'
    from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/rubygems/custom_require.rb:31:in `require'
    from /Users/kalabiyau/.rvm/gems/ruby-1.9.3-p385/gems/git-review-1.1.6/bin/git-review:6
    from /Users/kalabiyau/.rvm/gems/ruby-1.9.3-p385/bin/git-review:19:in `load'
    from /Users/kalabiyau/.rvm/gems/ruby-1.9.3-p385/bin/git-review:19
    from /Users/kalabiyau/.rvm/gems/ruby-1.9.3-p385/bin/ruby_noexec_wrapper:14

i will investigate and fix it

This actually a GitHub bug, which I already reported. Just waiting for the fix.

Even though the headless state should remain the default option for the 'checkout' command, it would be nice to have a flag (e.g. '--branch') to allow the creation of a local copy of the remote branch. This would enable the reviewer to push additional commits to the remote branch and thus to add to the reviewed code.

When closing a request without merging it, there appears to be a bug that leads to unwanted output. Even though everything is working fine, it prints "Request 'x' does not exist."

I have export TARGET_BRANCH in my .bashrc in order to not have to type it repeatedly. In order to run a prepare I have to unset it.

Steps to create:

export TARGET_BRANCH=branch
git review prepare

I just tried to create a new pull request with the latest trunk version of git-review. Unfortunately, it failed.

bamboo@ignis :: _files/code/trash ‹review_130830_more› > ../git-review/bin/git-review create
Everything up-to-date
commits: 1
["I love more than one cookie\n", "\n"]
POST https://api.github.com/repos/b4mboo/trash/pulls: 404: Not Found

The latest release (1.1.6) works well:

bamboo@ignis :: _files/code/trash ‹review_130830_more› > git review create          
Everything up-to-date
commits: 1
["I love more than one cookie\n", "\n"]
Switched to branch 'master'
Successfully created new request #45
https://github.com/b4mboo/trash/pull/45
Switched to branch 'review_130830_more'

I'm not quite sure if that might be connected to calling the binary like this. I need to investigate...

Whenever you want to clean up your repository's branches and run git review clean --all there are a couple of left over branches, which have commits that have not yet been merged to master. We are looking for an easy way to know who might still need that branch or whether the contents are obsolete. In order to identify the one who created that branch, you can of course take a look at the commits (and that is probably the only safe way to handle these things). However, it would be nice to have the git username in the branch name to tell the respective team member to take care of that.

The idea is to simply put the git username into the prefix, which git-review uses to identify review branches. E.g. review_130514_b4mboo_feature_name.

For a branch name containing a slash, like jr/devise, I get an error when checking out a pull request:

# bash 
$ git review checkout 39 --branch
Checking out changes to your local repository.
To get back to your original state, just run:

  git checkout master

error: pathspec 'jr/devise' did not match any file(s) known to git.

After calling git review create, it would be nice to return the ID of the new requst to the user. That way it is a lot easier to ask for a review.

When running git review show ID, it says

fatal: Invalid symmetric difference expression HEAD...85e5797790c31e4bb655da78228af2fc06cdeadd

I have found that this is because of the fetch that wasn't happening because

"#{repo.owner}/#{repo.name}" if repo

returned nil.
The reason is simple, it shouldn't be repo = request.head.repository but it should be repo = request.head.repo.
I'm working on the fix, the pull request is coming.

I recently upgraded get-review on my machine to version 0.8.6. An attempt to use it ended up with an error:

dmajda@dreadnought:~/dev/studio/ui-server> git review create
/usr/lib64/ruby/gems/1.8/gems/octokit-0.5.1/lib/octokit.rb:16:in `method_missing': undefined method `api_version=' for Octokit:Module (NoMethodError)
    from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.6/bin/../lib/git-review.rb:341:in `configure_github_access'
    from /usr/lib64/ruby/gems/1.8/gems/octokit-0.5.1/lib/octokit/configuration.rb:41:in `configure'
    from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.6/bin/../lib/git-review.rb:340:in `configure_github_access'
    from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.6/bin/../lib/git-review.rb:189:in `initialize'
    from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.6/bin/git-review:8:in `new'
    from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.6/bin/git-review:8
    from /usr/bin/git-review:19:in `load'
    from /usr/bin/git-review:19

After upgrading Octokit to version 0.6.5, the problem was solved. The git-review code apparently used a functionality available only in a newer version than the one I had installed.

I looked at git-review.gemspec and noted that required dependency versions are not specified there at all. They should be specified explicitly using >= or ~= so that issues like I encountered do not happen and upgrades of dependencies are forced when needed.

Currently git-review requires octokit version 0.5.1. It would be better to be less strict with this checks, since this is an old version of octokit.

Let the user choose to enter a personal description to the request. If the user chooses to leave it empty, add all commit messages' titles to the request body to be able to get an overview of the included changes.

Pending:
  Commands #prepare when on master/target branch moves uncommitted changes to the new branch
    # Temporarily disabled with xit
    # ./spec/git-review/commands_spec.rb:243

We should aim to get this running again as soon as possible or (if obsolete) remove it completely.

Incorporating something like this: https://github.com/jehiah/git-open-pull/blob/master/git-open-pull would make it easier to work off of existing issues without having to create a duplicate pull request issue.

When creating a new request enable the user to set a 'trivial' flag, such that a potential reviewer can follow the new SUSE Studio deployment process and merge it autonomously if he agrees with the original developer on the triviality of the patch.

It would be nice to be able to create review requests not only against master, but against any arbitrary branch. This would enable developers to work together on a more complex feature on a remote branch.

Enable the user to display existing comments and add new ones.

Right now the default behavior is to checkout the PR to a headless state. By using the optional switch --branch you can get git-review to actually create a local branch. I'd like to change that behavior to have the branch by default and instead provide a switch --no-branch or --headless in case you don't want the local branch.

Since git-review does not support working on forks (yet), we need to output an error message explaining this circumstance to the user, in case he tries to use git-review on a fork. Otherwise the error he will receive is pretty cryptic.

/usr/lib64/ruby/gems/1.8/gems/multi_json-1.0.3/lib/multi_json/engines/yajl.rb:10:in parse': lexical error: invalid char in json text. (MultiJson::DecodeError) <!DOCTYPE html> <html> <head> (right here) ------^ from /usr/lib64/ruby/gems/1.8/gems/multi_json-1.0.3/lib/multi_json/engines/yajl.rb:10:indecode'
from /usr/lib64/ruby/gems/1.8/gems/multi_json-1.0.3/lib/multi_json.rb:65:in decode' from /usr/lib64/ruby/gems/1.8/gems/faraday_middleware-0.7.0/lib/faraday/response/parse_json.rb:16:inparse'
from /usr/lib64/ruby/gems/1.8/gems/faraday-0.7.5/lib/faraday/response.rb:17:in on_complete' from /usr/lib64/ruby/gems/1.8/gems/faraday-0.7.5/lib/faraday/response.rb:9:incall'
from /usr/lib64/ruby/gems/1.8/gems/faraday-0.7.5/lib/faraday/response.rb:62:in on_complete' from /usr/lib64/ruby/gems/1.8/gems/faraday-0.7.5/lib/faraday/response.rb:8:incall'
from /usr/lib64/ruby/gems/1.8/gems/faraday-0.7.5/lib/faraday/response.rb:8:in call' from /usr/lib64/ruby/gems/1.8/gems/faraday-0.7.5/lib/faraday/request/url_encoded.rb:14:incall'
from /usr/lib64/ruby/gems/1.8/gems/faraday-0.7.5/lib/faraday/connection.rb:207:in run_request' from /usr/lib64/ruby/gems/1.8/gems/faraday-0.7.5/lib/faraday/connection.rb:94:inpost'
from /usr/lib64/ruby/gems/1.8/gems/octokit-0.6.5/lib/octokit/request.rb:28:in send' from /usr/lib64/ruby/gems/1.8/gems/octokit-0.6.5/lib/octokit/request.rb:28:inrequest'
from /usr/lib64/ruby/gems/1.8/gems/octokit-0.6.5/lib/octokit/request.rb:18:in post' from /usr/lib64/ruby/gems/1.8/gems/octokit-0.6.5/lib/octokit.rb:18:insend'
from /usr/lib64/ruby/gems/1.8/gems/octokit-0.6.5/lib/octokit.rb:18:in method_missing' from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.5/bin/../lib/git-review.rb:104:inclose'
from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.5/bin/../lib/git-review.rb:191:in send' from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.5/bin/../lib/git-review.rb:191:ininitialize'
from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.5/bin/git-review:8:in new' from /usr/lib64/ruby/gems/1.8/gems/git-review-0.8.5/bin/git-review:8 from /usr/bin/git-review:19:inload'
from /usr/bin/git-review:19

• forked from master
• pushed PR with one commit
  PR description shows huge set of changes

@b4mboo know more about it that me, because i never took a proper look to git-review internals.

Write tests to cover the existing code.

steps to reproduce:

• Linux machine
• git review prepare
• enter name for branch
• hit backspace

admin^?^?^?^?^?sdsd^?^?^?^?s^?

version 1.1.6

I think it could be nice to have a man page for git-review.
I've seen it's possible to do with the gem-man gem so that

gem man git-review

displays the man page.
What do you think ?