Bug 201956 - Python 3: Add support in webkitpy.common.checkout
Summary: Python 3: Add support in webkitpy.common.checkout
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-09-18 17:02 PDT by Jonathan Bedard
Modified: 2019-10-25 17:00 PDT (History)
9 users (show)

See Also:


Attachments
Patch (12.79 KB, patch)
2019-10-25 08:16 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (12.81 KB, patch)
2019-10-25 11:14 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch for landing (13.33 KB, patch)
2019-10-25 11:52 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-09-18 17:02:46 PDT
Many scripts depend on code in webkitpy.common.checkout (there are also some cyclical dependencies with webkitpy.common.system), so we need to have support here soon.
Comment 1 Radar WebKit Bug Importer 2019-09-18 17:04:22 PDT
<rdar://problem/55499884>
Comment 2 Jonathan Bedard 2019-10-25 08:16:12 PDT
Created attachment 381925 [details]
Patch
Comment 3 dewei_zhu 2019-10-25 10:53:07 PDT
Comment on attachment 381925 [details]
Patch

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

> Tools/ChangeLog:20
> +        (Contributor.__hash__): Committers need to be washable.

You mean 'hashable'?

> Tools/Scripts/webkitpy/common/config/committers.py:82
> +        for email in self._case_preserved_emails:

Why we would like to use case preserved emails? Does it mean a@webkit.org and A@webkit.org are considered as different contributors? Also, we may want to explain why we need this function now in change log.
Comment 4 Jonathan Bedard 2019-10-25 11:09:44 PDT
Comment on attachment 381925 [details]
Patch

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

>> Tools/Scripts/webkitpy/common/config/committers.py:82
>> +        for email in self._case_preserved_emails:
> 
> Why we would like to use case preserved emails? Does it mean a@webkit.org and A@webkit.org are considered as different contributors? Also, we may want to explain why we need this function now in change log.

This was a bit arbitrary, there is a case for me using what we do in __eq__, but that seems excessive. We need it so that we can put it into a set, I'll add that to the changelog.
Comment 5 Jonathan Bedard 2019-10-25 11:14:43 PDT
Created attachment 381945 [details]
Patch
Comment 6 dewei_zhu 2019-10-25 11:20:21 PDT
Comment on attachment 381945 [details]
Patch

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

> Tools/Scripts/webkitpy/common/config/committers.py:84
> +    def __hash__(self):
> +        result = hash(self.full_name)
> +        for email in self._case_preserved_emails:
> +            result ^= hash(email)
> +        return result

Discussed with Jonathan in person, we think we should use all fields used in __eq__ to compute hash.
Comment 7 Jonathan Bedard 2019-10-25 11:52:54 PDT
Created attachment 381951 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2019-10-25 17:00:43 PDT
Comment on attachment 381951 [details]
Patch for landing

Clearing flags on attachment: 381951

Committed r251613: <https://trac.webkit.org/changeset/251613>
Comment 9 WebKit Commit Bot 2019-10-25 17:00:44 PDT
All reviewed patches have been landed.  Closing bug.