W3C home > Mailing lists > Public > public-test-infra@w3.org > April to June 2020

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

From: Philip Jägenstedt <foolip@chromium.org>
Date: Tue, 19 May 2020 23:32:03 +0200
Message-ID: <CAARdPYfTngJxrG9mrHzK4GHM-foLmv5Yx-0gTUWFLW9A43Fgfw@mail.gmail.com>
To: Stephen Mcgruer <smcgruer@chromium.org>
Cc: Robert Ma <robertma@chromium.org>, Josh Matthews <josh@joshmatthews.net>, Ziran Sun <zsun@igalia.com>, James Graham <james@hoppipolla.co.uk>, Anne van Kesteren <annevk@annevk.nl>, public-test-infra <public-test-infra@w3.org>, ecosystem-infra <ecosystem-infra@chromium.org>
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>

> 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 Tuesday, 19 May 2020 21:32:33 UTC

This archive was generated by hypermail 2.4.0 : Tuesday, 19 May 2020 21:32:33 UTC