Bug 191928 - [ews-app] Add methods to save patch to database
Summary: [ews-app] Add methods to save patch to database
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-23 16:17 PST by Aakash Jain
Modified: 2018-11-26 16:25 PST (History)
5 users (show)

See Also:

Attachments
Proposed patch (3.40 KB, patch)
2018-11-23 18:57 PST, Aakash Jain 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2018-11-23 16:17:33 PST 
We should add methods in ews/models/patch.py allowing to save a Patch to database after performing necessary verification on input data.
Comment 1 Aakash Jain 2018-11-23 18:57:25 PST 
Created attachment 355546 [details]
Proposed patch
Comment 2 Json 2018-11-26 12:31:55 PST 
Comment on attachment 355546 [details]
Proposed patch

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

lgtm, other than the refactor of patchid to patch_id.

> Tools/BuildSlaveSupport/ews-app/ews/models/patch.py:45
> +    def save_patch(cls, patchid, bug_id=-1, obsolete=False):

can we use patch_id instead of patchid ?

> Tools/BuildSlaveSupport/ews-app/ews/models/patch.py:63
> +        if not patchid or type(patchid) != int or patchid < 0:

rare edge case...but could we have a patchid of 0?  maybe this should be "patchid < 1" ?
Comment 3 Lucas Forschler 2018-11-26 12:45:39 PST 
Comment on attachment 355546 [details]
Proposed patch

lgtm, other than the refactor of patchid to patch_id.
Comment 4 Kocsen Chung 2018-11-26 14:45:33 PST 
Comment on attachment 355546 [details]
Proposed patch

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

>> Tools/BuildSlaveSupport/ews-app/ews/models/patch.py:45
>> +    def save_patch(cls, patchid, bug_id=-1, obsolete=False):
> 
> can we use patch_id instead of patchid ?

++
Comment 5 Aakash Jain 2018-11-26 14:52:25 PST 
> can we use patch_id instead of patchid ?
Sure, will use patch_id.

> rare edge case...but could we have a patchid of 0?  maybe this should be "patchid < 1" ?
Sure, will change.
Comment 6 Aakash Jain 2018-11-26 16:24:06 PST 
Committed r238529: <http://trac.webkit.org/changeset/238529>
Comment 7 Radar WebKit Bug Importer 2018-11-26 16:25:30 PST 
<rdar://problem/46260973>