PAYRE NATHAN p1508475 writes:
>>> +sub parse_header_line {
>>> + my $lines = shift;
>>> + my $parsed_line = shift;
>>> + my $pattern = join "|", qw(To Cc Bcc);
>>
>> Nit: you may want to rename it to something more explicit, like
>> $addr_headers_pat.
The existing code mixes parsing of email header with regular
expression and actual code. Extract the parsing code into a new
subroutine "parse_header_line()". This improves the code readability
and make parse_header_line reusable in other place.
"parsed_header_line()" and "filter_body()" could be
"PAYRE NATHAN p1508475" wrote:
> - print $c2 $_;
> }
> +
> close $c;
Nit: this added newline does not seem necessary to me. Nothing
serious, but this kind of thing tend to distract the reader when
reviewing the patch.
> + foreach my
The existing code mixes parsing of email header with regular
expression and actual code. Extract the parsing code into a new
subroutine "parse_header_line()". This improves the code readability
and make parse_header_line reusable in other place.
"parsed_header_line()" and "filter_body()" could be
2017-12-11 22:12 GMT+01:00 Matthieu Moy :
> "PAYRE NATHAN p1508475" wrote:
>> + my %parsed_email;
>> + $parsed_email{'body'} = '';
>> + while (my $line = <$c>) {
>> + next if $line =~ m/^GIT:/;
>> +
"PAYRE NATHAN p1508475" wrote:
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -685,7 +685,7 @@ Lines beginning in "GIT:" will be removed.
> Consider including an overall diffstat or table of contents
> for the patch you are writing.
>
> -Clear
The existing code mixes parsing of email header with regular
expression and actual code. Extract the parsing code into a new
subroutine "parse_header_line()". This improves the code readability
and make parse_header_line reusable in other place.
"parsed_header_line()" and "filter_body()" could be
Nathan PAYRE writes:
> I've tested your code, and after few changes it's works perfectly!
> The code looks better now.
> Thanks a lot for your review.
Thanks, both of you.
Could you send in the final version later so that I can pick it up?
I agree with Matthieu's
I've tested your code, and after few changes it's works perfectly!
The code looks better now.
Thanks a lot for your review.
2017-12-03 23:00 GMT+01:00 Ævar Arnfjörð Bjarmason :
>
> On Sat, Dec 02 2017, Payre Nathan jotted:
>
>> From: Nathan Payre
>>
>>
On Sat, Dec 02 2017, Payre Nathan jotted:
> From: Nathan Payre
>
> The existing code mixes parsing of email header with regular
> expression and actual code. Extract the parsing code into a new
> subroutine 'parse_header_line()'. This improves the code readability
> and
Nathan PAYRE writes:
> I found a mistake in my signed-off-by, please replace
> by
I think you want exactly the opposite of this. You're contributing as a
Lyon 1 student, hence your identity is @etu.univ-lyon1.fr.
I found a mistake in my signed-off-by, please replace
by
Excuse me.
2017-12-02 18:02 GMT+01:00 Payre Nathan :
> From: Nathan Payre
>
> The existing code mixes parsing of email header with
From: Nathan Payre
The existing code mixes parsing of email header with regular
expression and actual code. Extract the parsing code into a new
subroutine 'parse_header_line()'. This improves the code readability
and make parse_header_line reusable in other place.
13 matches
Mail list logo