Vasi code review

README.md

@@ -12,12 +12,12 @@ cd REDMINE_ROOT/plugins
 git clone https://github.com/dergachev/redmine_git_remote
 ```
 
-Then enable the new GitRemote SMC type in [http://redmine-root/settings?tab=repositories](http://redmine-root/settings?tab=repositories)
+Then enable the new GitRemote SCM type in [http://redmine-root/settings?tab=repositories](http://redmine-root/settings?tab=repositories)
 
 ![](https://dl.dropbox.com/u/29440342/screenshots/AYKNZDTB-2014.11.27-15-59-06.png)
 
 Be sure to install the appropriate SSH keys to `~/.ssh/id_rsa` (for your redmine user).
-I recommend creating a dedicated deployment user on github/gitlab for this purpose.
+I recommend creating a [dedicated machine user](https://developer.github.com/guides/managing-deploy-keys/#machine-users) on github/gitlab for this purpose.
 
 ## Usage
 
@@ -28,17 +28,18 @@ GitRemote, enter the clone URL. The identifier and path will be auto-generated,
 ![](https://dl.dropbox.com/u/29440342/screenshots/ATIAQXHG-2014.11.27-15-03-51.png)
 
 On submitting the repository creation form, the identifier and `url`
-(filesystem path) fields will be auto-generated (if not explicitly provided) as follows:
+(filesystem path) fields will be auto-generated (if not explicitly provided).
 
-* Clone URL: `https://github.com/dergachev/vagrant-vbox-snapshot`
-* URL (filesystem path): `REDMINE_PLUGINS_PATH/redmine_git_remote/repos/github.com/dergachev/vagrant-vbox-snapshot`
+For example, if you enter `https://github.com/dergachev/vagrant-vbox-snapshot` as the Clone URL,
+it will prefill the Identifier and filesystem path fields as follows:
 * Identifier: `vagrant-vbox-snapshot`
+* URL (filesystem path): `REDMINE_PLUGINS_PATH/redmine_git_remote/repos/github.com/dergachev/vagrant-vbox-snapshot`
 
-Once the remote URL is validated, the plugin creates an "empty clone" at the specified path.
+Once the remote URL is validated, the plugin creates an [empty clone](http://stackoverflow.com/questions/895819/whats-the-most-straightforward-way-to-clone-an-empty-bare-git-repository) at the specified path.
 
 This plugin hooks into the core `Repository.fetch_changesets` to automatically
-run `git fetch --all` on all GitRemote managed repositories, before those
-commits are imported into Redmine. 
+run `git fetch --all` on all GitRemote managed repositories as Redmine is about
+to pull in changesets from the local repos.
 
 To avoid slowing down the GUI, we recommend unchecking the "Fetch commits
 automatically" setting at

app/models/repository/git_remote.rb

@@ -52,7 +52,7 @@ class Repository::GitRemote < Repository::Git
     self.identifier = p[:identifier] if identifier.empty?
     self.url = PATH_PREFIX + p[:path] if url.empty?
 
-    err = clone_empty
+    err = ensure_possibly_empty_clone_exists
     errors.add :extra_clone_url, err if err 
   end
 
@@ -61,7 +61,7 @@ class Repository::GitRemote < Repository::Git
     a.chomp.gsub(/\/$/,'') == b.chomp.gsub(/\/$/,'')
   end
 
-  def clone_empty
+  def ensure_possibly_empty_clone_exists
     Repository::GitRemote.add_known_host(clone_host)
 
     unless system "git ls-remote -h #{clone_url}"
@@ -109,7 +109,7 @@ class Repository::GitRemote < Repository::Git
     puts "Fetching repo #{clone_path}"
     Repository::GitRemote.add_known_host(clone_host)
 
-    err = clone_empty
+    err = ensure_possibly_empty_clone_exists
     Rails.logger.warn err if err
 
     # If dir exists and non-empty, should be safe to 'git fetch'

init.rb

@@ -1,16 +1,10 @@
 require 'redmine'
-
-# TODO: why isn't this autoloaded? 
-# NB: at this point, $PATH only contains {PLUGINS}/lib and app/models, app/controllers
-#     but not {PLUGINS}/app/models. Maybe those get added later?
-require File.dirname(__FILE__) + '/app/models/repository/git_remote'
-
-require_dependency "repository_fetch/repositories_helper_patch"
+require_dependency "redmine_git_remote/repositories_helper_patch"
 
 Redmine::Scm::Base.add "GitRemote"
 
 Redmine::Plugin.register :redmine_git_remote do
-  name 'Repository Fetch'
+  name 'Redmine Git Remote'
   author 'Alex Dergachev'
   url 'https://github.com/dergachev/redmine_git_remote'
   description 'Automatically clone and fetch remote git repositories'

lib/redmine_git_remote/repositories_helper_patch.rb

@@ -1,4 +1,4 @@
-module RepositoryFetch
+module RedmineGitRemote
   module RepositoriesHelperPatch
     def self.included(base) # :nodoc:
       base.send(:include, InstanceMethods)
@@ -6,17 +6,14 @@ module RepositoryFetch
 
     module InstanceMethods
       def git_remote_field_tags(form, repository)
-        #TODO: change URL label to "Path"
-        content_tag('p', form.text_field(:url, :size => 60, :required => true,
-                           :disabled => !repository.safe_attribute?('url'), :required => false) +
-                          content_tag('em', 
-                            l(:text_git_remote_path_note),
-                            :class => 'info') +
+        content_tag('p', form.text_field(:url,
+                           :size => 60, :required => true, :required => false,
+                           :disabled => !repository.safe_attribute?('url'),
+                           :label => l(:field_path_to_repository)) +
+                          content_tag('em', l(:text_git_remote_path_note), :class => 'info') +
                           form.text_field(:extra_clone_url, :size => 60, :required => true,
                            :disabled => !repository.safe_attribute?('url')) +
-                          content_tag('em', 
-                            l(:text_git_remote_url_note),
-                            :class => 'info')
+                          content_tag('em', l(:text_git_remote_url_note), :class => 'info')
                    )
       end
     end