Rename issue status "RTBC"

I support this, but let's wait for a day or two if somebody comes up with a better suggestion.

Weak start but....

• Ready To Be Considered
• Reviews To Be Considered

As dww said, I was trying to come up with something that matched the existing RTBC so that the zillion posts and articles that already use RTBC still have "meaning".

patch (needs Review Trusted By Committers)
patch (Request for Thoughts By Committers)
patch (Reviewed Thoroughly, Baby! Commit!) (j/k)
patch (Reviewed Thoroughly, Bidding Committers)
patch (Reviewed, needs Testing By Committers)
patch (Requesting Test By Committers)
patch (Requires Testing By Committers)

The last two hit the acronym perfectly, but don’t implicitly describe where they fall in the workflow. (i.e. if I just wrote a patch, can I get the committers to test it?) The ones that start with ”Reviewed, …” show the workflow better.

Perhaps, “patch (reviewed, Requesting Trial By Committers)”?

we don't concern ourselves with breaking backwards compatibility with apis so why should we worry about our issue status names ;) RFCR!

Ready for Committer Review sounds great to me.

As far as I can tell there were two main problems this development cycle:

1. Feedback for some patches came too late after too much work was done (this is why we are considering something along the lines of a 'concept needs review' status)

2. Most people are trying to rush patches through by saying they are ready to be committed before they are properly tested. And, worse, once they they are set as ready, being reluctant to set back (even when they raise an issue).

The new terminology will not help people be more stern with their reviews. And it will not encourage people to look at patches after they are set to their 'ready for committer review' status. In fact I'd argue this could make that issue worse, since we are referring to the committers to the exclusion of everyone else.

The thing that is more likely to improve these issues is the fact that we have had that dev list discussion and hopefully the culture has changed slightly as a result. I believe Eaton, for example, said he was going to start being harsher with his reviews. This attitude will filter down and become part of the culture of how things are done. Your actions as leaders in the queues will have a much bigger impact than the changing of a few words.

It is your actions and responses that set the expectations.

It doesn't matter what you decide to change it to, 4 words will always be misinterpreted.

-1

Alan, the 2 issues you mention are, indeed, important problems in the latest development cycle. Please look at http://groups.drupal.org/node/4970 where there is a more general discussion about proposed fixes for core development issues.

This thread doesn’t attempt to address the 2 issues you mention. It’s simply trying to re-align developers expectations as to what happens when something is marked RTBC. The “Ready to Be Committed” wording leads to too many assumptions on relatively-new contributors. “Ready for committer review” (or something like it) better explains the process to those contributors.

As for your relevant comment that this change…

…will not encourage people to look at patches after they are set to their 'ready for committer review' status. In fact I'd argue this could make that issue worse, since we are referring to the committers to the exclusion of everyone else.

That’s probably true. Instead of “Ready for Committer Review” should we use “Ready for Final Review”? That might make it clearer that everyone (and not just a committer) is welcome to give it a review.

My point is really that this seems like a non-issue to me. Patches get marked ready to be committed because someone thinks they are ready to be committed. The wording, as far as I can tell is perfectly accurate. Sure newcomers may misinterpret it, but they will misinterpret any wording you choose. That's what happens when you only have half a sentence to explain something. Newcomers just need to spend a little time getting acquainted with the system. This is why we have now added a section to the handbook explaining how all this works.

So,

I don't believe this will relieve any confusion for newcomers.

I do believe this will create some short-term confusion for people who already know the system.

"Feelings become hurt, arguments come about on the list and this recommendation is aimed to help prevent that."

This is why I brought up the two issues in comment #9. Those seem to be the cause of confusion and hurt feelings... not this. I just haven't seen any evidence that there is a causal link between the current wording and any problems.

I've already raised an issue with 'ready for committer review'. There is another critique about an alternate 'ready for final review'. And I think there will be problems with whatever wording we choose.

I just haven't seen any evidence that there is a causal link between the current wording and any problems.

Alan, the consensus on the devel list was that RTBC does need to be renamed. I invite you to read the “RTBC - how does it work?” thread on http://lists.drupal.org/pipermail/development/2007-July/thread.html Let’s not rehash the argument in this thread.

‘Ready for Final Review’ gives a connotative meaning that someone will give a final review before doing something else. The something else isn't specified but can be assumed to be either commit the patch or mark it as PNW.

Earnie, introducing a little uncertainty into the wording should be fine; making it more likely that newcomers will look up the meaning of RFFR would be a GOOD thing.

Thinking on this some more, improving the (necessarily-short) wording of the issue states is a good idea, but having a full description of the issue states readily available would be a really, REALLY good idea. The best thing would be a #description inside the Issue Information fieldset that links to the meaning of all the statuses and the Drupal development process. For example:

Descriptions of the Priority and Status levels can be found in the Contributing to Development Handbook.

Since the “submission guidelines” feature isn‘t exactly what I’m describing, I’ve added a feature request in the project_issue module (#159457) What do others think of this?

Getting back to this specific proposal, I prefer “Ready for Final Review” since it doesn‘t refer to the committers to the exclusion of everyone else. But “Ready for Committer Review” is still way better than RTBC.

Getting back to this specific proposal, I prefer “Ready for Final Review” since it doesn‘t refer to the committers to the exclusion of everyone else. But “Ready for Committer Review”

I think I prefer "Ready for Final Review" as well (although both are an improvement).

Sometimes patches that need work or concepts that need approval have already had a review from a committer.

Ready for Final Review suggests that the patch has stabilised and is ready for a final once over, not just by a committer. It's a good indication of the status of the patch, and again it suggests that it'll either get committed or be set to PCNW - which is what actually happens to most RTBC patches (and will hopefully happen quicker in both directions as things move forward).

This renaming is indeed badly needed. I really like JohnAlbin's proposals in #6. However, I'd suggest to clarify:

- We want neither not-yet-reviewed nor untested patches to be set to RTBC.
- Developers should only set RTBC if at least one other developer reviewed and tested a patch.
- RTBC is the last step in the workflow in front of committing a patch.

So, I would propose this:

patch (code needs review)
patch (code needs work)
patch (reviewed and tested by others)

Thus, RTBC would become RTBO and that status is totally clear to everyone.

Note: patch (reviewed & tested by others) would be cleaner, but AFAIK project_issue does not support ampersands.

If we went with "reviewed & tested by others" we might consider "reviewed & tested by communtity" (RTBC) :) Although core commiters are certainly part of the community... Maybe too creative.

"reviewed & tested by communtity" i like this one a lot , as it reflects , very nicely, what status the issue should be in.

Another +1 for "reviewed & tested by communtity".

@earnie: Attaching polls/votes to file attachments is something to consider for D8 and beyond... anyway, that's not the point of this issue.

I don't think we need hard numbers here. "Community" is plural already. Thus, from my understanding it would indicate that a patch needs to be reviewed AND tested by at least two contributors.
Furthermore, when submitting a patch in a new issue, I would not "be able" to set the issue status to RTBC, because *I know* it hasn't been reviewed, nor tested by anyone else. And that's one of the points of this issue.

+1 for "reviewed & tested by community"...

...but shouldn't we still have "ready to be committed"? I'm thinking about my contrib modules, and I think it would work the same way in core. If a committer type reviews something after "reviewed & tested by community", couldn't it move to another state before being committed? I'm just thinking of the times I've reviewed something, liked it, but wasn't planning on committing it just yet. So maybe what we want to do here is add a new status instead of renaming an existing one.

In other words, ""reviewed & tested by community" would be the status before ""ready to be committed". The problem then, of course, would be that one of those acronyms would have to change. ;)

//Thus, from my understanding it would indicate that a patch needs to be reviewed AND tested by at least two contributors.

That's how I understand it as well. And a poll doesn't say this IMO. Meaningful comments in followups do.

Another +1 from me.

Cross posted with colan, but that's a good point about contrib. Could we rename all current RTBC issues to "Reviewed and tested by community" then make a new status "To be committed" which is only available to users with cvs access to a project? I don't know how the permissions work on this (probably not like that though).

I agree. “reviewed & tested by community” fits very nicely. Thanks for the suggestion, Anders!

Ok. Given that we’ve got 3 reviews of Anders’ suggestion, I’m marking this RTBC.

These docs will need updating after the change: http://drupal.org/node/156119

Could we rename all current RTBC issues to "Reviewed and tested by community" then make a new status "To be committed" which is only available to users with cvs access to a project?

If only users with cvs access can set it to "to be committed". Why not just commit it and mark it “fixed”? :-)

@colan, is the list of RTBC issues so long in your contrib module that you need a separate status just to track the ones you want to commit, but later? You can just post a comment saying "I'll commit this soon."

JohnAlbin - yeah good point. I see very few RTBC patches in contrib so it's perhaps not much of an issue.

ouch! Do we have a string limit here? I only see "patch (reviewed and tested by co" :-/

“patch (reviewed and tested by co” is 32 characters.

Anybody want to write a patch to increase the limit? :-)

Aaand... action!

Doh.

Started an issue @ http://drupal.org/node/224986

Oops. Too late. :D

Sigh. ;) Resetting all statuses.

Personally, I don't think #224986: Make issue status field longer should have been marked as a duplicate, because the length of the status field is a project_issue problem, but the name of the status really belongs in the webmasters queue, unless the proposal is that the project issue module itself should change the name of that status, which I don't think is a good idea.

Since the patch here only addresses the length of the status field, I'm re-opening #224986: Make issue status field longer and moving this back to the webmasters queue.

http://drupal.org/node/156119 has been updated with new RTBC definition. yay!

I still think putting a link to the Priority and Status definitions inside the “Edit issue settings” fieldset is a good idea. Feature request: http://drupal.org/node/159457

For example:

Descriptions of the Priority and Status levels can be found in the Contributing to Development Handbook.