Additional labels for pull requests (and Trac tickets)

[Mojca Miklavec]

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