Re: Porting wptserve handlers to Python 3: next steps / code review

> Maybe we could just have wpt-pr-bot not assign any reviewers for Ziran's
PRs at all? Even simpler :)

... I don't get how that's simpler? As long as Ziran choses an assignee,
wpt-pr-bot shouldn't* override that or touch the assignee field, so it all
should be fine?

Or are you looking to avoid folks in META.yml being cc'd at all?

* Ok, wpt-pr-bot is a bit buggy, so it might :/

On Tue, 19 May 2020 at 17:32, Philip Jägenstedt <foolip@chromium.org> wrote:

> Maybe we could just have wpt-pr-bot not assign any reviewers for Ziran's
> PRs at all? Even simpler :)
>
> On Tue, May 19, 2020 at 9:54 PM Stephen Mcgruer <smcgruer@chromium.org>
> wrote:
>
>> I think it suffices in this case to just have a known list of users, and
>> Ziran can round-robin between them as she sends out pull requests.
>> Automation seems overkill here :)
>>
>> On Tue, 19 May 2020 at 15:53, Philip Jägenstedt <foolip@chromium.org>
>> wrote:
>>
>>> Attempting to answer the question for Stephen since I've poked a bit at
>>> wpt-pr-bot in the past.
>>>
>>> In
>>> https://github.com/web-platform-tests/wpt-pr-bot/blob/master/lib/comment.js we
>>> have access to the inferred labels (as metadata.labels) and could use them,
>>> but the issue is that the python3 label is now added by hand. One could try
>>> to add support for acting on labels that are added at PR creation time, or
>>> reaction to added labels, but it the resulting behavior would not be easy
>>> to understand, I predict.
>>>
>>> I guess there's no way to infer from a PRs content alone if it's a
>>> python3 conversion?
>>>
>>> On Fri, May 15, 2020 at 7:41 PM Robert Ma <robertma@chromium.org> wrote:
>>>
>>>> Thank you, Josh & Philip!
>>>>
>>>> I've updated the RFC
>>>> <https://github.com/web-platform-tests/rfcs/pull/49> to reflect our
>>>> latest consensus (preferring bytes everywhere). Please take another look
>>>> (the diff
>>>> <https://github.com/web-platform-tests/rfcs/pull/49/commits/2e95208da921d31cfbd38a37204777bd62ef1fd1>
>>>> isn't big because I basically swapped two sections with some additions).
>>>>
>>>> Stephen, would it be possible to tweak wpt-pr-bot to handle PRs with
>>>> the "python3" label and assign them to a special pool of reviewers?
>>>>
>>>> On Wed, May 13, 2020 at 8:17 AM Philip Jägenstedt <foolip@chromium.org>
>>>> wrote:
>>>>
>>>>> I can confirm I volunteer to review!
>>>>>
>>>>> On Tue, May 12, 2020 at 8:54 PM Josh Matthews <josh@joshmatthews.net>
>>>>> wrote:
>>>>>
>>>>>> I've got some experience porting the eventsource handlers (although I
>>>>>> haven't submitted that PR yet due to a couple unfinished tricky handlers),
>>>>>> so I can review PRs as well.
>>>>>>
>>>>>> Cheers,
>>>>>> Josh
>>>>>>
>>>>>> On Tue, 12 May 2020 at 13:03, Stephen Mcgruer <smcgruer@chromium.org>
>>>>>> wrote:
>>>>>>
>>>>>>> Thanks for sending this email Robert; I'm excited to see us keep the
>>>>>>> ball rolling on Python 3 support.
>>>>>>>
>>>>>>> > In addition, we'd really appreciate a few more people to sign up
>>>>>>> for reviewing these changes to share the workload. Anyone volunteering?
>>>>>>>
>>>>>>> I'm happy to review PRs, albeit with no specific prior knowledge.
>>>>>>> +foolip, who volunteered to review as well.
>>>>>>>
>>>>>>> That'd bring us to 5 reviewers assuming jgraham and annevk are
>>>>>>> willing to review; do you think that is enough Robert?
>>>>>>>
>>>>>>> On Mon, 11 May 2020 at 18:19, Robert Ma <robertma@chromium.org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hello everyone,
>>>>>>>>
>>>>>>>> We're getting close to finalizing the plan for migrating close to
>>>>>>>> 500 wptserve handlers we have in WPT. Now we have a few concrete steps to
>>>>>>>> take:
>>>>>>>>
>>>>>>>> 1. Regarding the trial PR
>>>>>>>> <https://github.com/web-platform-tests/wpt/pull/23363>, James,
>>>>>>>> Anne and others who'd like to take a look, do you have any other comments
>>>>>>>> on this PR, especially high-level ones about the general approach? This
>>>>>>>> would unblock the following steps and we can address small issues in
>>>>>>>> parallel.
>>>>>>>> 2. If we agree this approach is what we wanted by having consistent
>>>>>>>> and explicit semantics across Python 2 and 3, I'll update the RFC
>>>>>>>> <https://github.com/web-platform-tests/rfcs/pull/49> (essentially
>>>>>>>> swapping the currently "recommended" and "alternative" approaches and
>>>>>>>> filling in some more concrete guidelines), and kick off a new round of RFC
>>>>>>>> process (hopefully relatively quick since many people are already on board
>>>>>>>> with the new approach).
>>>>>>>> 3. Meanwhile, Ziran can start porting more handlers (we can wait
>>>>>>>> until the RFC is accepted to actually merge the PRs). We have hundreds of
>>>>>>>> handlers and we should expect lots of PRs. Reviewing them is a critical
>>>>>>>> task, too. Since we now have concrete guidelines and changes will be
>>>>>>>> largely mechanical, I'm proposing to adopt the "LGTM % nits" convention
>>>>>>>> widely used in Chromium: if a PR largely looks good but has some minor
>>>>>>>> issues, approve the PR with comments. In addition, we'd really appreciate a
>>>>>>>> few more people to sign up for reviewing these changes to share the
>>>>>>>> workload. Anyone volunteering?
>>>>>>>>
>>>>>>>> Best,
>>>>>>>> Robert
>>>>>>>>
>>>>>>>

Received on Wednesday, 20 May 2020 11:38:44 UTC