Skip to contentSkip to conversation

git-add--interactive.perl: Remove two unused variables

PATCH In review Thomas "Enki" Badie 3 participants 7 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 git-add--interactive.perl: Remove two unused variables

The patch 8f0bef6 refactors this script and leaves the $fh' variable unused in diff_applies' and `patch_update_file'.

---
Hello,
This is my first patch. It introduces a really minor change, and I also want to be sure the way I submit it is the right way. Thanks :)

 git-add--interactive.perl |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
git-add--interactive.perl +0 -2
index d948aa8..710764a 100755
@@ -1067,7 +1067,6 @@ EOF
 }
 
 sub diff_applies {
-       my $fh;
       return run_git_apply($patch_mode_flavour{APPLY_CHECK} . '--check',
                            map { @{$_->{TEXT}} } @_);
 }
@@ -1514,7 +1513,6 @@ sub patch_update_file {
       }
 
       if (@result) {
-               my $fh;
               my @patch = reassemble_patch($head->{TEXT}, @result);
               my $apply_routine = $patch_mode_flavour{APPLY};
               &$apply_routine(@patch);
Signature · 1 line
1.7.5.4
#1 lore raw
Quoted 8 lines · Thomas \"Enki\" Badie

The patch 8f0bef6 refactors this script and leaves the $fh' variable unused in diff_applies' and `patch_update_file'.

---
Hello,
This is my first patch. It introduces a really minor change, and I also want to be sure the way I submit it is the right way. Thanks :)

The patch submission is almost perfect, except that:

(1) the patch seems to be whitespace damaged; and

(2) the author of the problematic commit should have been Cc'ed (especially when he is still an active participant on the list) to give him a chance to Ack it (I'm adding Thomas for this).

Thanks. I like that you identified why this is a good thing by quoting the problematic change.

Quoted 23 lines
 git-add--interactive.perl |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
git-add--interactive.perl +0 -2
index d948aa8..710764a 100755
@@ -1067,7 +1067,6 @@ EOF
 }
 
 sub diff_applies {
-       my $fh;
       return run_git_apply($patch_mode_flavour{APPLY_CHECK} . '--check',
                            map { @{$_->{TEXT}} } @_);
 }
@@ -1514,7 +1513,6 @@ sub patch_update_file {
       }
 
       if (@result) {
-               my $fh;
               my @patch = reassemble_patch($head->{TEXT}, @result);
               my $apply_routine = $patch_mode_flavour{APPLY};
               &$apply_routine(@patch);
Acked-by
#2 lore raw
Quoted 4 lines · Junio C Hamano
Quoted 2 lines · Thomas \"Enki\" Badie

The patch 8f0bef6 refactors this script and leaves the $fh' variable unused in diff_applies' and `patch_update_file'.

[...]

Quoted 3 lines

(2) the author of the problematic commit should have been Cc'ed (especially when he is still an active participant on the list) to give him a chance to Ack it (I'm adding Thomas for this).

Indeed, my bad. It's easy to verify from 'git show 8f0bef6' that this was an oversight in my patch.

Signature · 2 lines
Thomas Rast
trast@{inf,student}.ethz.ch
#3 lore raw

2012/6/25 Thomas Rast <[email protected]>:

Quoted 7 lines
Quoted 4 lines · Junio C Hamano
Quoted 2 lines · Thomas \"Enki\" Badie

The patch 8f0bef6 refactors this script and leaves the $fh' variable unused in diff_applies' and `patch_update_file'.

[...]

Quoted 8 lines
Quoted 3 lines

 (2) the author of the problematic commit should have been Cc'ed (especially when he is still an active participant on the list) to give him a chance to Ack it (I'm adding Thomas for this).

Indeed, my bad.  It's easy to verify from 'git show 8f0bef6' that this was an oversight in my patch.

It happens to everyone^^ Thanks for your answer.

Should I repost a patch V2 or it is okay like this? Thanks

Signature · 1 line
Thomas "Enki" Badie
#4 lore raw
In reply to Junio C Hamano

2012/6/25 Junio C Hamano <[email protected]>:

Quoted 11 lines
Quoted 8 lines · Thomas \"Enki\" Badie

The patch 8f0bef6 refactors this script and leaves the $fh' variable unused in diff_applies' and `patch_update_file'.

---
Hello,
This is my first patch. It introduces a really minor change, and I also want to be sure the way I submit it is the right way. Thanks :)

Quoted 4 lines

The patch submission is almost perfect, except that:

 (1) the patch seems to be whitespace damaged; and

The only damage I see is a space on the deleted lines which doesn't exist in the patch. Is that you're talking about ?
I need to investigate why it appears...

Quoted 3 lines

 (2) the author of the problematic commit should have been Cc'ed (especially when he is still an active participant on the list) to give him a chance to Ack it (I'm adding Thomas for this).

My bad, I forgot to check if the commiter were still active. I'll check it for the next time.

Quoted 2 lines

Thanks.  I like that you identified why this is a good thing by quoting the problematic change.

Thanks :)

Quoted 23 lines
Quoted 23 lines

 git-add--interactive.perl |    2 --

 1 files changed, 0 insertions(+), 2 deletions(-)
git-add--interactive.perl +0 -2
index d948aa8..710764a 100755
@@ -1067,7 +1067,6 @@ EOF
  }
 
  sub diff_applies {
-      my $fh;
        return run_git_apply($patch_mode_flavour{APPLY_CHECK} . '--check',
                             map { @{$_->{TEXT}} } @_);
  }
@@ -1514,7 +1513,6 @@ sub patch_update_file {
        }
 
        if (@result) {
-              my $fh;
                my @patch = reassemble_patch($head->{TEXT}, @result);
                my $apply_routine = $patch_mode_flavour{APPLY};
                &$apply_routine(@patch);
Signature · 1 line
Thomas "Enki" Badie
#5 lore raw
Quoted 12 lines · Thomas Badie

2012/6/25 Junio C Hamano <[email protected]>:

Quoted 11 lines
Quoted 8 lines · Thomas \"Enki\" Badie

The patch 8f0bef6 refactors this script and leaves the $fh' variable unused in diff_applies' and `patch_update_file'.

---
Hello,
This is my first patch. It introduces a really minor change, and I also want to be sure the way I submit it is the right way. Thanks :)

Quoted 8 lines
Quoted 4 lines

The patch submission is almost perfect, except that:

(1) the patch seems to be whitespace damaged; and

The only damage I see is a space on the deleted lines which doesn't exist in the patch. Is that you're talking about ?
I need to investigate why it appears...

A typical context line would begin with SP and typically then tab (because our code is indented with leading tabs) but your context lines all have two SPs in front and indented with more SPs. This is a symptom of your MUA mangling your patch, and the reason can be seen in the "Content-Type: text/plain; charset=ISO-8859-1; format=flowed" header.

#6 lore raw
Quoted 23 lines · Junio C Hamano
Quoted 12 lines · Thomas Badie

2012/6/25 Junio C Hamano <[email protected]>:

Quoted 11 lines
Quoted 8 lines · Thomas \"Enki\" Badie

The patch 8f0bef6 refactors this script and leaves the $fh' variable unused in diff_applies' and `patch_update_file'.

---
Hello,
This is my first patch. It introduces a really minor change, and I also want to be sure the way I submit it is the right way. Thanks :)

Quoted 8 lines
Quoted 4 lines

The patch submission is almost perfect, except that:

(1) the patch seems to be whitespace damaged; and

The only damage I see is a space on the deleted lines which doesn't exist in the patch. Is that you're talking about ?
I need to investigate why it appears...

Quoted 7 lines

A typical context line would begin with SP and typically then tab (because our code is indented with leading tabs) but your context lines all have two SPs in front and indented with more SPs. This is a symptom of your MUA mangling your patch, and the reason can be seen in the "Content-Type: text/plain; charset=ISO-8859-1; format=flowed" header.

I follow the first approach in the tutorial in
git format-patch --help, and I hope it works as expected :)

I wonder what is the next step, should I repost the previous patch with the line "Acked-by: Thomas Rast <[email protected]>" or it is okay right now?

Thanks a lot for your help on this first patch,

Signature · 1 line
Thomas "Enki" Badie