Redmine

(for example, tortoisesvn client has no svn executable), so you must recommend to install SVN server with standard svn.exe available and which supports standard SVN commands (like tortoise svn does not)

Actually it is included, since TortoisSVN is only an interface for the command line client, and afaik also added to the path.
Other than that note, it's a great idea.

Patch is based on "a" and "b" project paths, could you make it based on project root ?

At first glance, I'd say that code is neat and that "SCM" title should not be the same size than "Information".

Just a thought : SCM information could also be located in / linked to the Administration/Settings/Repositories/activated SCM list if you plan to change its behavior so that the administrator immediately understand the reason he can't activate such scm.

I'll test it later.

Got a HTTP 500 rendering admin/info :

SCM command failed, make sure that your SCM binary (eg. svn) is in PATH (...): "darcs" --version
  with: No such file or directory - "darcs" --version

Redmine::Scm::Adapters::CommandFailed (No such file or directory - "darcs" --version):
  lib/redmine/scm/adapters/abstract_adapter.rb:211:in `shellout'
  lib/redmine/scm/adapters/darcs_adapter.rb:49:in `darcs_binary_version_from_command_line'
  lib/redmine/scm/adapters/darcs_adapter.rb:42:in `darcs_binary_version'
  lib/redmine/scm/adapters/darcs_adapter.rb:38:in `client_version'
  lib/redmine/scm/adapters/abstract_adapter.rb:41:in `client_version_string'
  app/models/repository.rb:225:in `scm_version_string'
  app/controllers/admin_controller.rb:90:in `info'
  app/controllers/admin_controller.rb:86:in `each'
  app/controllers/admin_controller.rb:86:in `info'

The only SCM installed is Subversion (and FS, of course).

Toshi MARUYAMA wrote:

Thank you for reviewing.

This is select list patch.

Can't you just disable the checkboxes ?

Etienne Massip wrote:

Toshi MARUYAMA wrote:

Thank you for reviewing.

This is select list patch.

Can't you just disable the checkboxes ?

I am not good at HTML coding. Could you try?

Etienne Massip wrote:

Got a HTTP 500 rendering admin/info :

[...]

The only SCM installed is Subversion (and FS, of course).

Japan is midnight.
I will fix tomorrow.

Good night then =)

Sorry, I was mistaken : I thought the 2nd patch was for the Administration/Settings/Repositories/activated SCM checkboxes list.

Handling checkboxes state in this list would require some changes in setting_multiselect() in source:trunk/app/helpers/settings_helper.rb (like adding a yield block returning a map of additional options ?) and probably also a bit in source:trunk/app/views/settings/_repositories.rhtml.

About the select list patch, going in the project Settings tab throws a HTTP 500 error too, still in the shellout() method this time called by hg adapter.

I fixed in r4882.

Both works fine now.

My remarks :

• "SCM" title should be in lower size than section title "Information"
• column titles and column values are not well justified

You should handle checkboxes state in screen "Administration/Settings/Repositories/activated SCM" depending upon SCM availability, as stated in my previous notes.

Why not do this control in SCM availability selection in Administration/Settings instead of Project settings, since the SCM list in Project settings is already filtered depending upon this selection in Administration/Settings ?

Etienne Massip wrote:

Why not do this control in SCM availability selection in Administration/Settings instead of Project settings, since the SCM list in Project settings is already filtered depending upon this selection in Administration/Settings ?

Default list (exclude filesystem) is default setting.
There is no chance to check whether SCM commands are available or not.

And, it is difficult in case of machine replacing or recovering from database.

Toshi MARUYAMA wrote:

Default list (exclude filesystem) is default setting.
There is no chance to check whether SCM commands are available or not.

But the whole point of a feature request is to add a feature.
Denying it because it's not the current functionality is counter-intuitive.

Moreover, actually, project managers who are not admins will see the command path, this is odd.

But you did commit some changes to trunk / 1.2, didn't you ?

The followings are FAQ.

• TortoiseSVN does not have svn.exe
• /usr/local/bin/git is not PATH for web server
• hg.cmd and bzr.bat can not be used.

So, I think this feature is beneficial to administrators and project managers.

It really makes more sense on the administration panel. Toshi, please update this ticket as soon as it is done, 1.2 is about to be released.

Jean-Philippe Lang wrote:

It really makes more sense on the administration panel. Toshi, please update this ticket as soon as it is done, 1.2 is about to be released.

I see.

I have committed in trunk.
Please review it.

Jean-Philippe Lang wrote:

Looks much better, thanks :-) I've just made a few UI changes in r5960.
I think we can close it now.

Thanks, I will merge to 1.2-stable.

Merged in 1.2-stable in r5961.

Jean-Philippe Lang wrote:

Merged in 1.2-stable in r5961.

Thanks!!