On Sun, 11 Apr 2010, Damien Tournoud wrote:
> On Sun, Apr 11, 2010 at 7:33 PM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> > To not break backward compatibility, wouldn't it be better to check if
> > elements of @git_base_url_list end with ':' or '/', and join base with
> > project path depending on this condition, i.e.:
> >
> > + @url_list = map { m/[/:]$/ ? "$_$project" : "$_/$project" } @git_base_url_list
> > + unless @url_list;
> >
> > This means: if base ends with colon ':' or slash '/', concatenate base
> > and project path, otherwise join them using '/' as field separator.
>
> Thanks for the review. Indeed this way sounds better.
>
> All this is new to me so I'm not sure what is the way forward. Should
> I publish another patch or would you?
If you don't mind, I'd rather you send updated patch (perhaps as response
to this email, or staring a separate thread).
Note that you would need to:
1. Perhaps update first line of commit message, i.e. email subject.
But current version might be good enough; if you don't change the
subject you should use [PATCH v2] rather than [PATCH] in it.
2. Update commit message, to say that you are modifying the way project
URLs shown in gitweb are composed out of bases in @git_base_url_list
and $project, to allow possibility of generating scp-like relative
URL for SSH access, e.g. 'git@xxxxxxxxxxxxxxx:user/repo.git'
3. You might want to put comment about what changed from previous
version of the patch between "---\n" and diffstat.
4. You would need to update description of @git_base_url_list in
gitweb/README, e.g. providing example what to put in @git_base_url_list
to get 'git@xxxxxxxxxxxxxxx:user/repo.git': it is 'git@xxxxxxxxxxxxxxx:'
5. Update comment in gitweb.perl above declaration of @git_base_url_list
to follow modified way of getting URL from base and $project
6. Make above change in git_summary.
P.S. Thank you for contributing to git.
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html