Additional labels for pull requests (and Trac tickets)

Fri Nov 17 07:20:39 UTC 2017

Hi,

Does anyone else have any additional thoughts regarding potential new flags?

waitingfor:review
    nobody with sufficient expertise took a look yet to provide
    some qualified feedback; alternatively this ticket needs
    more opinions about which way to go
    (not sure if there's a special tag needed for that)
waitingfor:maintainer
    someone from our team thinks the commit is ok, but would prefer
    to wait for the port maintainer to confirm the change
waitingfor:trivial_polish
    maybe commit message needs to be fixed, commits squashed or split
    into more atomic units, or other straightforward changes

waitingfor:incomplete/needs_more_work
    the changes are not quite ready yet and fixing them is not
    straightforward
<prefix>:broken
    this change breaks something (say, an upgrade to boost or lua
    that would break a number of dependent ports)

waiting:upstream
    could refer to:
    - waiting for upstream to fix something
    - waiting for upstream to at least provide some basic feedback
      about the patch (see (a) below)
    - waiting for the submitter to at least file a ticket to an
      upstream bug tracker and reference that ticket (see (b))

helpneeded

We could potentially add tags with the meaning:
    "easy", "medium difficulty", "difficult".
In that case the above suggestions could be rearranged a bit (say,
needs_more_work could be a single flag, but then the level of
difficulty would tell whether it's a trivial change or heavy rework
that's needed).

Mojca

PS: Regarding upstream:

(a) Someone submitting a controversial patch that nobody in our
community even understands (other than the author of the patch). In
such cases I would prefer to get at least some very basic understand
of what upstream feels about the patch. Are they ready to accept it
immediately? Would they patch it in a different way? Do they think the
patch would break some fundamental functionality? ...

(b) It might actually be some simple and straightfoward patch. It
might even be backported from upstream, but with no references. I
would like the author to either reference an existing bug/commit or
submit a new one if one doesn't exist.

On 11 November 2017 at 23:20, Mojca Miklavec <mojca at macports.org> wrote:
> On 11 November 2017 at 21:36, Jeremy Lavergne wrote:
>> Perhaps we can give these all a common prefix since they appear to fit a
>> theme: wait/waiting/hold.
>
> Yes, I wanted to suggest a common prefix and a common colour indeed.
>
>> Assuming it can be helpful if one can search
>> tags themselves so everything that's "waiting-*" shows up, or at the
>> very least get a list of tags to then view.
>>
>> Think we're also on track for indicating who owns the next step:
>>  * reporter (them)
>
> In our case it's rather the submitter/author of the patch than
> reporter, but the person we have in mind is the same.
>
>>  * macports/maintainer (us)
>
> Two different scenarios need to be considered here. In one case it's
> any developer. In the other it's the maintainer.
>
>>  * third party (other)
>
> This could be two different actions:
> - Either we asked the author to submit a bug report and want him/her
> to at least provide a link to the bug report.
> - Or we are in fact waiting for upstream to do something.
> But I would nevertheless use the same tag for both.
>
> I like your analysis and proposal in general.
>
>> On 11/11/2017 02:35 PM, Mojca Miklavec wrote:
>>> On 11 November 2017 at 20:04, Jeremy Lavergne wrote:
>>>>> - needs_review: nobody with sufficient expertise took a look yet to
>>>>> provide some qualified feedback
>>> This is kind of default state anyway. But some PRs might be extra
>>> tricky to do and I would reserve the tag for those.
>>
>> Agreed: it draws specific/extra attention to needing a/additional
>> review. Do you feel it's distinct from "wait_for_opinion" or perhaps
>> they can be combined?
>
> From the conceptual point of view one tag should suffice. I imagined
> that "needs a review" would call for a highly skilled technical
> reviewer. But on the second thought: some controversial issues that
> "don't require learning a new technical skill or spending hours" to do
> it, but can in principle be done in a few seconds, they just need
> someone to actually take a look and *decide* ... I would also probably
> call those "needs review".
>
> Maybe a single tag is in fact better. Difficult problems could
> potentially later be marked as "help_needed" or "difficult" :)
>
>
>>>>> - wait_for_maintainer: someone from our team thinks the commit is ok,
>>>>> but would prefer to wait for the port maintainer to take a look first
>>> This is also kind-of-evident from:
>>> - a green check given by some reviewer (but it's never clear whether
>>> this check was given by the actual maintainer or someone else; this
>>> would make it more clear)
>>> - a request for the maintainer to make a review
>>> but it would be nice to have an obvious label
>>>
>>> This would probably be most useful for Trac.
>>
>> Hoping for clarity on the Trac implication.
>>
>> Does this mean we need to copy/reference the issue from GitHub in a
>> ticket in Trac if we want specific devs to notice action is requested?
>
> No. If we want our devs to notice GitHub PRs, we should first and
> foremost fill in the gaps with missing GitHub handles is Portfiles.
> Then Zero King's script will notify maintainers automatically. But
> that's something that could to a large extent be done automatically.
>
>
> What I meant (without ever saying that loud) is the following.
>
> We currently have > 30 PRs open on GitHub. Last time I checked we had
> roughly 5k open issues on Trac. Coping with those 30+ PRs on GitHub is
> somewhat manageable even with no extra tags, but I would really like
> to find out the best way to keep those numbers down. We still stand a
> chance to actually close most of them one day. So I started
> brainstorming on how to best handle issues on GitHub.
>
> Once we are done brainstorming PRs, we should start brainstorming ways
> to cope with those 5k(?) Trac tickets. That will be a lot more work in
> any case, but some tags could be similar to those introduced on
> GitHub.
>
>> Curious if our "process" has a hole, that devs aren't available on
>> GitHub and so we have to do extra work on tickets to fill the gap.
>
> No, I didn't mean any cross-referencing here.
>
>> Can you further explain what you meant? Don't want to have myself go on
>> accidental tangents :-)
>
> See above.
>
>>>>> - more_changes_needed: the changes are not quite ready yet
>>> A while ago I created a tag WIP and people argued that
>>> work-in-progress should not even be submitted as a PR. It was then
>>> removed. I would still find it useful though. It might turn out that
>>> 90% of work is done and someone finds a problem that needs to be
>>> resolved first.
>>>
>>> There's another potentially related thing (not sure if it should be
>>> the same or another tag). Like "no, please don't commit an update to
>>> lua because this breaks many ports". Or, "wait-a-moment, it turns out
>>> that something is broken after this, I need a couple of more days to
>>> fix it".
>>
>> This group appears to be more of "incomplete" or "do not act on this",
>> right? If it's going to break many things, that could be seen as arguing
>> it's incomplete. Tempted to say keep these combined.
>
> These can be combined of course. I can imagine a red tag screaming to
> you: "even if you are bored and have nothing better to do, please
> don't be tempted to simply press the merge button" (at least not until
> something happens, but you probably need to carefully read and fully
> understand what's going on).
>
>>>>> - wait_for_upstream_feedback: we would prefer if upstream would take a
>>>>> look, or at least to get an upstream ticket open before merging the
>>>>> changes
>>
>> Worried about an issue lingering in GitHub. Is upstream only invoked if
>> a maintainer made contact, and can we expect the maintainer to have
>> already determined the course of action regardless of upstream involvement?
>
> Let me explain what I imagine this flag being used for most frequently:
>
> (a) Someone submitting a controversial patch that nobody in our
> community even understands (other than the author of the patch). In
> such cases I would prefer to get at least some very basic understand
> of what upstream feels about the patch. Are they ready to accept it
> immediately? Would they patch it in a different way? Do they think the
> patch would break some fundamental functionality? ...
>
> (b) It might actually be some simple and straightfoward patch. It
> might even be backported from upstream, but with no references. I
> would like the author to either reference an existing bug/commit or
> submit a new one if one doesn't exist.
>
> The idea of "upstream" tag is not to impose impossible conditions for
> ever accepting the contribution. It should be there to encourage the
> author to talk to upstream if that didn't happen already.
>
> If upstream is impossible to reach or is too conservative in accepting
> patches, this should not stop us from applying patches locally.
>
>
>
> The same tag ("upstream") in a bug report would mean the difference in
> prioritizing the bug. For the bug submitter it means: "please go and
> keep pushing upstream developers to fix this, we are not experts in
> fixing someone else's bugs". For the developers it means much lower
> priority. Our efforts should concentrate in making sure that our base
> works as well as possible and that we fix our own bugs that nobody
> else can fix instead of us, while the "upstream" bugs will disappear
> as soon as someone else familiar with that piece of code fixes the
> problem.
>
>>>>> - wait_for_opinion: there's still an ongoing debate, opinions
>>>>> potentially differ, we need more brainstorming etc.
>
> Mojca