docwilmot
commented
3 years ago I dont agree that we can have both labels on an issue. If it is RTBC (in either interpretation of the acronym) it means it ready, which can only mean that the code is satisfactory and its been tested. I believe how I've been viewing this (and I suspect most others) is that we have two intermediate labels:
works for me: I've tested it but I havent reviewed the codeneeds testing: I havent tested it; I may or may not have reviewed the code.
But RTBC means I've done both.
olafgrabienski
stpaultim
I am not sure if this is the best place for this discussion, but I'll give it a try.
I'd like more clarity on how folks are using the "RTBC" label in the core issue queue and then suggest that we update the "Contribution" page to be a bit more clear.
I've seen some issues that are marked both "PR-Ready To Be Committed" and "PR - Needs Testing".
This seems contradictory to me, since how can something be ready to be committed if it still needs testing. I've always assumed the following:
1) That you can't really do a final code review until after an issue has been tested. I often see folks do a preliminary code review and add a comment saying that the "Code looks good to me." I usually treat that as a provisional code review that applies only if no problems are found in testing and no changes to the PR. 2) That an issue is not "RTBC" until it has been both successfully tested and the code reviewed.
Neither of the following sources of documentation (both of which talk about the RTBC label) are completely clear on this issue.
https://docs.backdropcms.org/documentation/contribute-to-backdrop-core https://backdropcms.org/user-guide/adding-labels-issues
The second source of documentation does say:
pr - ready to be committed = Someone who did not create the pull request will need to test and review the work. If this person approves of the pull request, they can then add this label.This seems to imply that testing and code review are both complete.