threads / patch / 11565

patchgitweb: Change feeds from commit to commitdiff output.

Subject: [PATCH] gitweb: Change feeds from commit to commitdiff output.

## tl;dr

4 messages between Jan 10, 2008 and Jan 11, 2008. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Florian La Roche· Jan 10, 2008, 13:09 UTC · lore

[PATCH] gitweb: Change feeds from commit to commitdiff output.

Change feeds from displaying the commit to displaying the commitdiff output. This way the patches are shown directly and code review is done more easily via watching feeds.

Signed-off-by: Florian La Roche <laroche@redhat.com>
---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to gitweb/gitweb.perl +1 −1

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 6256641..0db83c0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5566,7 +5566,7 @@ XML
 			or next;
 
 		# print element (entry, item)
-		my $co_url = href(-full=>1, action=>"commit", hash=>$commit);
+		my $co_url = href(-full=>1, action=>"commitdiff", hash=>$commit);
 		if ($format eq 'rss') {
 			print "<item>\n" .
 			      "<title>" . esc_html($co{'title'}) . "</title>\n" .

-- 
1.5.3.8

Junio C Hamano· Jan 10, 2008, 20:21 UTC · re: Florian La Roche · lore

Re: [PATCH] gitweb: Change feeds from commit to commitdiff output.

Florian La Roche <laroche@redhat.com> writes:

Show 7 quoted lines

> [PATCH] gitweb: Change feeds from commit to commitdiff output.
>
> Change feeds from displaying the commit to displaying the commitdiff
> output. This way the patches are shown directly and code review is
> done more easily via watching feeds.
>
> Signed-off-by: Florian La Roche <laroche@redhat.com>

I can see that easier access to commitdiff output is sometimes desirable.

If you are making this change unconditional, however, I think there needs a list discussion between you and the silent majority of people that have been perfectly happy with the current "log only" behaviour.

And to have that discussion, you first have to wake them up, which this patch would serve well as a wake-up call. But if that was the purpose of the posting, please (1) mark the patch as such (commonly done by saying [RFC/PATCH] instead), and (2) keep me out of the "To:" list, if the patch is not for inclusion but for discussion ("cc:" is fine but it's redundant as long as you are sending to the list).

After such a discussion, we may end up finding out that everybody have been silently unhappy and wanted to have commitdiff there, and agree on doing this unconditionally.

But we do not know that yet.

Thanks.

Florian La Roche· Jan 11, 2008, 06:53 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: Change feeds from commit to commitdiff output.

On Thu, Jan 10, 2008 at 12:21:21PM -0800, Junio C Hamano wrote:

Show 17 quoted lines

> Florian La Roche <laroche@redhat.com> writes:
> 
> > [PATCH] gitweb: Change feeds from commit to commitdiff output.
> >
> > Change feeds from displaying the commit to displaying the commitdiff
> > output. This way the patches are shown directly and code review is
> > done more easily via watching feeds.
> >
> > Signed-off-by: Florian La Roche <laroche@redhat.com>
> 
> I can see that easier access to commitdiff output is sometimes
> desirable.
> 
> If you are making this change unconditional, however, I think
> there needs a list discussion between you and the silent
> majority of people that have been perfectly happy with the
> current "log only" behaviour.

Hello Junio,

Right, this is a change in behaviour. Maybe a config option for this would be good, so that users can configure their wanted output style.

Show 7 quoted lines

> And to have that discussion, you first have to wake them up,
> which this patch would serve well as a wake-up call.  But if
> that was the purpose of the posting, please (1) mark the patch
> as such (commonly done by saying [RFC/PATCH] instead), and (2)
> keep me out of the "To:" list, if the patch is not for inclusion
> but for discussion ("cc:" is fine but it's redundant as long as
> you are sending to the list).

Ok.

Show 5 quoted lines

> After such a discussion, we may end up finding out that
> everybody have been silently unhappy and wanted to have
> commitdiff there, and agree on doing this unconditionally.
> 
> But we do not know that yet.

>From asking people, most don't use feeds to follow development

checkins, but prefer to keep with mailinglists and looking only at the repos for their own interesting parts.

Hope more people will speak up here on the git@ list.

Thanks a lot,

Florian La Roche

Jakub Narebski· Jan 11, 2008, 11:12 UTC · re: Florian La Roche · lore

Re: [PATCH] gitweb: Change feeds from commit to commitdiff output.

Florian La Roche <laroche@redhat.com> writes:

Show 5 quoted lines

> [PATCH] gitweb: Change feeds from commit to commitdiff output.
> 
> Change feeds from displaying the commit to displaying the commitdiff
> output. This way the patches are shown directly and code review is
> done more easily via watching feeds.

First, this changes _link_, and not display (change summary), so the commit message IMHO should read rather:

	gitweb: Change feeds link from commit view to commitdiff view.
	
	Change feed link from link to commit view to link to commitdiff
	view. This way the patches are shown directly and code review is
	done more easily via watching feeds.

I personally do not use gitweb feeds not feed readers, so I cannot say if this change truly helps. I just don't know what the "feed watcher" workflow looks like.

Second, at least for Atom format it is possible to give both 'commit' and 'commitdiff' view links as alternate links for a feed. That is something to consider.

Show 9 quoted lines

> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5566,7 +5566,7 @@ XML
>  			or next;
>  
>  		# print element (entry, item)
> -		my $co_url = href(-full=>1, action=>"commit", hash=>$commit);
> +		my $co_url = href(-full=>1, action=>"commitdiff", hash=>$commit);
>  		if ($format eq 'rss') {

-- 
Jakub Narebski
Poland
ShadeHawk on #git