Jiri Moskovcak <[email protected]> writes:

> On 02/13/2012 12:03 PM, Nikola Pajkovsky wrote:
>> test 'Attaching better backtrace' is broken. problem dir contains
>> backtrace_rating 1 and servers response is backtrace_rating 3 ->>  never
>> upload.
>>
>> Signed-off-by: Nikola Pajkovsky<[email protected]>
>> ---
>>   src/plugins/reporter-bugzilla.c |   16 +++++++++-------
>>   src/plugins/rhbz.c              |   20 ++++++++++----------
>>   2 files changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/plugins/reporter-bugzilla.c 
>> b/src/plugins/reporter-bugzilla.c
>> index cd31f5b..c49ffe2 100644
>> --- a/src/plugins/reporter-bugzilla.c
>> +++ b/src/plugins/reporter-bugzilla.c
>> @@ -402,13 +402,14 @@ int main(int argc, char **argv)
>>                   strbuf_append_strf(full_desc, "%s: %s\n", FILENAME_RATING, 
>> rating_str);
>>               strbuf_append_strf(full_desc, "Package: %s\n", rhbz.b_package);
>>               /* attach the architecture only if it's different from the 
>> initial report */
>> -            if ((strcmp(bz->bi_platform, "All") != 0)&&
>> -                (strcmp(bz->bi_platform, "Unspecified") != 0)&&
>> -                (strcmp(bz->bi_platform, rhbz.b_arch) !=0))
>> +            if ((strcmp(bz->bi_platform, "All") != 0)
>> +&&  (strcmp(bz->bi_platform, "Unspecified") != 0)
>> +&&  (strcmp(bz->bi_platform, rhbz.b_arch) !=0))
>>                   strbuf_append_strf(full_desc, "Architecture: %s\n", 
>> rhbz.b_arch);
>>               else
>>               {
>> -                VERB3 log("not adding the arch: %s because rep_plat is %s", 
>> rhbz.b_arch, bz->bi_platform);
>> +                VERB3 log("not adding the arch: %s because rep_plat is %s",
>> +                      rhbz.b_arch, bz->bi_platform);
>>               }
>>               strbuf_append_strf(full_desc, "OS Release: %s\n", 
>> rhbz.b_release);
>>
>
> - just an indent fixes, please don't do that, it just ruins git
> history, these kind of problems should be noted at the review time,
> once it's pushed it's too late

hmm, ok

>> @@ -418,8 +419,8 @@ int main(int argc, char **argv)
>>                                                                         
>> "is_private");
>>               */
>>
>> -            int allow_comment = is_comment_dup(bz->bi_comments, 
>> full_desc->buf);
>> -            if (!allow_comment)
>> +            int dup_comment = is_comment_dup(bz->bi_comments, 
>> full_desc->buf);
>> +            if (!dup_comment)
>>               {
>>                   log(_("Adding new comment to bug %d"), bz->bi_id);
>>                   rhbz_add_comment(client, bz->bi_id, full_desc->buf, 0);
>> @@ -434,7 +435,8 @@ int main(int argc, char **argv)
>>               /* python doesn't have rating file */
>>               if (rating_str)
>>                   rating = xatou(rating_str);
>> -            if (!allow_comment&&  (bz->bi_best_bt_rating<  rating))
>> +
>> +            if (!dup_comment&&  (rating>  bz->bi_best_bt_rating))
>>               {
>
> - here it's just a variable rename, so the same as the above applies,
> but since the previous name was really wrong (wasn't that mine??) I
> will look away and let you push it :)

it's mine stupid variable name ;)

>>                   char bug_id_str[sizeof(int)*3 + 2];
>>                   sprintf(bug_id_str, "%i", bz->bi_id);
>> diff --git a/src/plugins/rhbz.c b/src/plugins/rhbz.c
>> index fbf5bba..ba88110 100644
>> --- a/src/plugins/rhbz.c
>> +++ b/src/plugins/rhbz.c
>> @@ -86,22 +86,22 @@ static char *trim_all_whitespace(const char *str)
>>       return trim;
>>   }
>>
>> -int is_comment_dup(GList *comments, const char *comment)
>> +int is_comment_dup(GList *srv_comments, const char *usr_comment)
>>   {
>> -    for (GList *l = comments; l; l = l->next)
>> +    char *trim_usr_comment = trim_all_whitespace(usr_comment);
>> +    for (GList *l = srv_comments; l; l = l->next)
>>       {
>> -        char *comment_body = (char *) l->data;
>> -        char *trim_comment_body = trim_all_whitespace(comment_body);
>> -        char *trim_comment = trim_all_whitespace(comment);
>> -        if (!strcmp(trim_comment_body, trim_comment))
>> +        char *srv_comment = (char *) l->data;
>> +        char *trim_srv_comment = trim_all_whitespace(srv_comment);
>> +        if (!strcmp(trim_srv_comment, trim_usr_comment))
>>           {
>> -            free(trim_comment_body);
>> -            free(trim_comment);
>> +            free(trim_usr_comment);
>> +            free(trim_srv_comment);
>>               return 1;
>>           }
>>       }
>>
>
> - and here it's really not that useful, so please don't do it...

it's way better to read, but I can push another dummy commit with
variable renaming

>> -    return 0;;
>> +    return 0;
>>   }
>>
>>   static unsigned find_best_bt_rating_in_comments(GList *comments)
>> @@ -122,7 +122,7 @@ static unsigned find_best_bt_rating_in_comments(GList 
>> *comments)
>>               continue;
>>           }
>>
>> -        start_rating_line += strlen("rating: ");
>> +        start_rating_line += strlen(FILENAME_RATING": ");
>
> - this is the actual fix? hm... looks too *easy* ;)

yes :)

>>           char *end_rating_line = strchr(start_rating_line, '\n');
>>           if (!end_rating_line)
>>               VERB3 error_msg("broken comment body");

-- 
Nikola

Reply via email to