To loosen a "10 days" grip of ZC on issues which have pull requests?

[skapral]

First realized this problem here and here.

I did the issue within 10 days and provided a PR. PR was successfully reviewed, remarks were applied and the fix is gone to upstream. However, from the moment PR gone to review till the moment when issue is closed ZC considers this issue not done by me (I still see ZC warning me that time is almost over). This makes the process too dependent on human factor:

1. Issue may be done within 10 days, but reviewer delayed his feedback (probably the positive one). ZC resignes the issue and developer's reputation is affected.

2. Issue may be done within 10 days, including PR review and merge. However, developer is not always able to close the issue by himself: his/her Github permissions may not be sufficient for that. It makes DEV dependent on PO or ARC (like in examples I provided), but ZC still considers the issue not done and can resign it.

My proposal is based on known principle "one task may have only one person responcible for it at a time". Maybe we can do some enhancements of ZC time tracking, like:

1. Adding explicit commands to DEV for linking the issue with the PR(s) solving it. Until the issue has merged or on-review PR(s), ZC doesn't count down 10 days for DEV. If review was declined with remarks, DEVeloper would have a time remaining from his 10 days for fixing the remarks.

2. Until review is not accepted or rejected, the responcibility for the issue is on REViewer, not on DEVeloper. If REViewer rejected PR, then DEVeloper can spend the time, remaining from 10 days on solving remarks and repeat review attempt. Once reviewer accepted the PR, it's ARC responcibility now. If ARC accepted the PR, it's PO responcibility now to close the issue.

3. Each role has their own time limit to get rid of responcibility. For DEV it is already defined - "10 days". What about REV, ARC and PO?

[0crat]

@yegor256/z please, pay attention to this issue

[nick318]

@yegor256 take a look, please

[yegor256]

@skapral it's a common problem, discussed many times. I do understand the motivation behind this proposal, but the 0/100 principle here must be very strict: a task is either closed or open. There is nothing in the middle. Zerocrat should not even try to understand what's going on or why the task is not yet closed. If we start paying attention to those details, developers will start to abuse the system, creating PRs, inventing excuses, etc.

It's 10 days. You can either finish it or you lose. That's it. Because the budgets are rather small, the damage you may occasionally get due to the "human factor" you mentioned is rather small.

[amihaiemil]

@yegor256

Because the budgets are rather small, the damage you may occasionally get due to the "human factor" you mentioned is rather small.

No, it's not that simple. Because so far, only the DEV suffers from this. All the "human factor" burdain is on the DEV, who can implement 20 tasks per day but have to wait at least another 3 days for everyone to do their part -- we can do nothing about this bottleneck, true, but add to that the fact that the DEV may wait 10+ days for all others to move and it's really frustrating.

Even if 0crat doesn't know anything and cannot smartly track everyone's time, there should be severe punishments for the person who delays the task so much that 3 other people get burned and lose their time.

Well, a very easy escape route is if I, as a DEV, simply put my tasks on hold... but I think there was an Issue where this should become a QA gate, QA will eventually complain if the DEV simply puts the task on hold without a "good" reason.

[llorllale]

@yegor256 @amihaiemil @skapral
I want to voice my concern by following up on @skapral 's comment:

Once reviewer accepted the PR, it's ARC responcibility now. If ARC accepted the PR, it's PO responcibility now to close the issue.

The fact that today it's possible for the PR to be merged and yet the DEV still may not be paid is a giant showstopper for me.

As DEV, this is my sacred, core principle:

If my PR gets merged, I should be paid. Period.

For me, the current workflow, point 7, and point 31 of the policy need to be amended to accommodate for this. Otherwise I'd rather just contribute to these projects voluntarily for free from outside Zerocracy.

[amihaiemil]

@llorllale I agree with you, but there is a little flaw in this principle:

If my PR gets merged, I should be paid, period.

A PR may be merged but it doesn't necessarily mean it solved the issue... it's only a little flaw since we've been practicing this for a lot of time and 99.9% of the time one PR was enough.

I say we enable this principle and see how we can solve the flaw later. It happened too little times in order to be a criterion for the decision, I would say.

Maybe we can even consider that always a PR is enough since the DEV doesn't have more time for it anyway. There are 30min after all. We should punish the ARC if the PR did not entirely fix the issue and also did not leave puzzles to continue implementation.

[skapral]

A PR may be merged but it doesn't necessarily mean it solved the issue...

Since all issues are estimated at 30 mins, hard to imagine for me the one which require more than one PR. Easier to restrict it then adopt IMO. Correct me if I wrong.

The fact that today it's possible for the PR to be merged and yet the DEV still may not be paid is a giant showstopper for me.

Good point. When I wrote an initial proposal, I came from the fact that neither 0crat, nor issue prime cannot close the issue. Usually only the architect and issue originator can do it, so the part "Once reviewer accepted the PR, it's ARC responcibility now. If ARC accepted the PR, it's PO responcibility now to close the issue" is hard to exclude.

[amihaiemil]

@skapral I also said the 30min stuff. So I think the following:

1. PR gets merged
2. DEV gets paid instantly
3. Issue author checks the solution and the solution can either complete or partial but with puzzles for continuing.
4. Issue author closes the Issue no matter what, but if the solution is not complete and there are also no puzzles, then the ARCH gets punished, since he/she allowed an incomplete PR to be merged. This decision could be taken by the QA, after the Issue's author complains to her/him that something is not ok with the solution.

[skapral]

If we start paying attention to those details, developers will start to abuse the system, creating PRs, inventing excuses, etc.

Generally, this can be avoided by combining two tricks:

1. Everyone has fixed time per issue for their responsibilities. It never enlarges. You may speculate it anyway, but eventually your time will come to the end.

2. (draft) To provide some penalty to DEV for constantly creating PRs with bad quality. This will prevent collusion between DEV and PR.

[amihaiemil]

@yegor256 I think you need to take into account these propsals. If there are no DEVs on the platform, there is no ZC :)

And this very strict 0/100 thing was always my discussion with all my friends whom I told about XDSD. As said above, if we have PDD, we can loosen the 0/100 rule and make everyone happy -- the project would also move a lot faster.

[skapral]

@amihaiemil as originator, I don't want this issue to become a flame. Until then all we have is a separate ideas. Let's summarize our ideas to some straight proposal, then call @yegor256.

[amihaiemil]

@skapral well, do you agree with what I said here? I agreed with @llorllale and with you. If you guys agree with that proposal of mine, then we're fine, right?

If so, just update the Issue's title to "Pay DEV instantly after PR is merged" and add as description my comment.

Let's wait for @llorllale (if you are fine with it)

[skapral]

I have no objections to you guys, yet, I just want to think it over, compare with current state of policy and provide the concrete one-comment proposal. For further objective review and discussion.

[llorllale]

@amihaiemil I agree but still wonder where does QA fit in your proposal?

[amihaiemil]

@llorllale I think QA should be the one to give the verdict to 0crat. Something like: "quality unacceptable" -- so, it's even lower than "bad"... "bad" is not cool, somebody will not be paid, but at least the project is not in an incosistent state. "unacceptable" means that that Issue has to be closed now (or assign to someone else, who won't know what is left to be done, because there are no puzzles) and the solution is not there.

QA would give this verdict after the Issue's author would complain: "hey, these guys screwed me over, they merged a PR, they got paid, they did not solve the problem completely and also they did not leave puzzles"

Well, this QA is just a propsal, the overall idea is still the same: pay the DEV on merged pull request and punish the ARC if the Issue's author complains.

[skapral]

@llorllale @amihaiemil BTW, don't you think that it is logical for QA to give the verdict even before the issue/PR are closed and everyone got their money, and lost the interest to fix anything?

Also, I have this issue about QA, JFYI: #305

[amihaiemil]

@skapral yes, maybe, but let's not discuss that now. It's another topic, not related to this already long thread :)

[llorllale]

@skapral that's what I was thinking: the QA review on the DEV should occur before the PR gets merged. However, @amihaiemil 's proposal does offer another way though: punish the ARC if he let a bad PR go through

[llorllale]

@amihaiemil I'm having trouble understanding your comment. Could you merge the steps it would entail with your proposal?

[amihaiemil]

@llorllale @skapral Yes, here it is, layed out (you can ignore my prev comments and propose this):

1. PR gets merged
2. DEV gets paid instantly
3. Author of Issue reviews the solution. If they are happy, all is fine, QA can say "quality good"
4. If author is not fine with solution (this means that it is incomplete and there are also no puzzles for continuing), then author tells QA to give the verdict "quality bad" -- bad is enough, no need for more gradients, as I said above, since DEV is already paid. In this case the ARC is punished.

If you are ok, I would say to open a new issue with the proposal (@skapral can do it) so we can discuss more there, if Yegor agrees. This thread is already too long.

[llorllale]

@amihaiemil agreed

[skapral]

@skapral that's what I was thinking: the QA review on the DEV should occur before the PR gets merged. However, @amihaiemil 's proposal does offer another way though: punish the ARC if he let a bad PR go through

@llorllale One thing is for sure: either QA should validate opened PR and give remarks, like they are doing it now, or they should validate closed stuff, but stop giving remarks and waiting for anything: just pull the "good/bad quality" trigger.

[llorllale]

@skapral I agree, but let's leave that topic for another ticket

[skapral]

@amihaiemil It is not enough. Closed PR but opened issue is not the only place which is dependent on human factor. What about PRs on review, with hanged response on some side?

[skapral]

@skapral I agree, but let's leave that topic for another ticket

@llorllale You are right, I'll create an issue for that.

[amihaiemil]

@skapral That is also another issue. We want to clearify the payment of the dev (loosen the 0/100 princple) -- hanged PR is for another issue as well, I would say. If PR is hanged then it's either really the DEV's fault or the CR's, or the ARCH's -- those are topics for new Issues, how to punish them properly.

[skapral]

@amihaiemil ok, I just outlined it so we don't forget the second part.

Speaking about your plan, I have a couple of concerns about point 4:

1. @llorllale outlined an open question about QA role in the process, and seems like the suitable place to clarify the answer is here. Until then, it is early to decide something.

2. You touched quite an interesting question which has not yet being discussed (probably worth a separate issue, let me know if we need it). What is the role of issue originator after the issue is gone to upstream? From one point of view, he/she can be like stakeholder, and reason about the quality like you mentioned in your proposal. But it would mean that he must review the issue solution together with QA. Not convenient, also nothing would motivate the originator to check the fix just in place immediately - he can screw QA activities. Or he/she may be like passive reporter - it's up to the reporter to provide exhaustive information about proposed feature or found bug, but once the issue is put in scope by the architect, then reporter is not in charge anymore. If the result doesn't met the reporter's expectations, it's subject to another issue.

[amihaiemil]

@skapral Yes, you are right, the Issue reporter (originator, as you call it), has no real motivation to check the solution. Well, I think you have 2 situations: 1 it is an outsider who may not care anymore about the project or 2 it is an insider, working in the project, who should care, since the fix will make their life easier as well.

Besides, it shouldn't be hard to check the solution: the bug reports are supposed to be as specific and concrete as possible. So you should see if the merged PR was ok in a few secconds (the PR is also supposed to change few files, I personally rejected PRs which had 15 changed files).

[skapral]

@amihaiemil

2 it is an insider, working with us, who should care, since they are working on the project, the fix will make their life easier as well.

Something tells me that even insider, according to @yegor256's paradigm of relations, is not supposed to care too much about it. Well - there is a chance that he cares, when he is personally interested in some fix. But it is definitely not an axiom to rely the policy on. And when he really cares, there may be a risk from originator's side to lobby some ideas through constantly causing ARC to be punished for "bad" quality ("bad" from originator's point of view). Not good.

Besides, it shouldn't be hard to check the solution

Doubt about that. First of all, not every issue reporter is supposed to be an expert in the project's code base, so looking into PR would say nothing to them. Then, each bug has certain preconditions to reproduce, which may be short in text description, but require lots of efforts to reproduce in reality. This may make the originator to postpone the fix validation till the next occasion, which is not in interest of QA according to your plan.

So, to sum up: I'd left the originator out of quality review, at least for now. It causes too much questions. The option to include him we may outline to a separate issue. The problem this issue was raised on is pretty covered with points 1-2 from your proposal + issue #317, isn't it?

[amihaiemil]

@skapral

The problem this issue was raised on is pretty covered with points 1-2 from your proposal + issue #317, isn't it?

Actually, yes, I think you are right. Indeed, involving Issue reported in QA matters is too complex for now.
So yes, we should let the QA decide, when reviewing the PR (before it is merged) if the solution is complete or not (and if it isn't complete, then there must be puzzles).

Can you open another ticket with points 1 and 2 and the QA stuff on the PR?

This thread is way too long now. Maybe you can just post a link in the new Issue, pointing to our discussion, for anyone who is interested.