Bug 200015 - resultsdbpy: Handle case where the previous commit doesn't have the changelog
Summary: resultsdbpy: Handle case where the previous commit doesn't have the changelog
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-22 14:42 PDT by Jonathan Bedard
Modified: 2019-07-22 16:48 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.56 KB, patch)
2019-07-22 14:50 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (2.08 KB, patch)
2019-07-22 15:56 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2019-07-22 14:42:15 PDT
Discovered this in a certain testing configuration. In practice, it's not actually a problem for real instances (at least right now) because we have to declare where all the changelogs are.
Comment 1 Jonathan Bedard 2019-07-22 14:50:00 PDT
Created attachment 374639 [details]
Patch
Comment 2 Aakash Jain 2019-07-22 15:37:42 PDT
Comment on attachment 374639 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374639&action=review

What happens if current commit doesn't have ChangeLog?

> Tools/resultsdbpy/resultsdbpy/model/repository.py:323
>                      if len(current_response.text) == (len(previous_response.text) if previous_response else 0):

can we simplify this if-else, this is super confusing. Especially if someone doesn't pay very close attention to the order of parenthesis, this doesn't make sense.

> Tools/resultsdbpy/resultsdbpy/model/repository.py:326
> +                    if not previous_response or len(current_response.text) < len(previous_response.text):

Maybe just add another variable previous_response_length and evaluate it beforehand, so as to simplify both the if-else. something like:

previous_response_length = 0
if previous_response:
     previous_response_length = len(previous_response.text)
Comment 3 Jonathan Bedard 2019-07-22 15:56:50 PDT
Created attachment 374645 [details]
Patch
Comment 4 WebKit Commit Bot 2019-07-22 16:47:22 PDT
Comment on attachment 374645 [details]
Patch

Clearing flags on attachment: 374645

Committed r247706: <https://trac.webkit.org/changeset/247706>
Comment 5 WebKit Commit Bot 2019-07-22 16:47:24 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2019-07-22 16:48:16 PDT
<rdar://problem/53423137>