remoteStorage

No easy way to see which pull requests are ready for review

right now when you look at https://github.com/remotestorage/remotestorage.js/pulls there are 5 pull requests, none of which are ready for review. On all of them, a reviewer has asked questions / given instructions of things that need changes, which either someone confirmed they’ll work on, or which are waiting for the OP to pick that up.

so now if you submit a new pull request, that you want people to review, how can you bring it to the reviewers’ attention?

we need some quick way to see which PRs need to be reviewed, so that reviewers can check that view once a day and ensure quick feedback to PR submitters.

i tried to establish this with the ‘ready for review’ label, but @raucao said he didn’t like it, and it looks like someone removed it from the labels list again.

for bonus points: would also be nice to have an easy way to see which issues are waiting for someone to pick them up.

so to maybe make a bit clearer what i mean, all PRs start in ‘ready for review’ state. then if someone writes comments which need attention, it drops out of ‘ready for review’ state, because the OP has work to do then. if someone gives a “+1” or a “+1” with just a few minor detail fixes, then it stays in ‘ready for review’ state, until a second person reviews it. it then leaves ‘ready for review’ state again, either because the OP has to fix the comments of the second reviewer, or because the OP or the second reviewer can merge & close it, so then it no longer requires the attention from more reviewers.

As I said on GitHub, it sounds like a lot of funky process and overhead to me. I haven’t had any problems so far with staying up to date just with notifications and reading the comments on the PRs. We only ever have 10 or so at the absolute most, usually more like 5.

Anyone else had the same problem as @michielbdejong? Or maybe ideas of how to solve this in a more straight-forward way, that hopefully doesn’t need a protocol for applying and removing tags in a specific way?

yeah, extra process steps are also not ideal…

we can also just write “ping” on any issue that someone sees has been stuck in reviewable state for > 72 hours. is that reasonable?

That’s basically what we’ve been doing, and I think it worked ok.

I also think the current process works quite alright.

Sometimes, you want to open a pull request early on although it’s not ready to merge yet. Maybe you are stuck or want some early feedback. For that I think it helps to add a task list to the description. For one others don’t point something out as missing because they can see you already planned for it. And the pull request overview also shows a helpful hint for them:

Great idea!

Sure the tasklist helps totally with the problem described by the OP.

But just to make you think about it: For the federated wiki, Ward has been playing with using their API and importing GitHub issues into a wiki page.

Sure this could be adapted to PRs, too, rendering it to some kind of alternative view of GitHub data.

Would there be any interest in seeing this?

Interested in seeing what exactly? I don’t understand how what you described relates to this topic.

I was talking about a Wiki importing GitHub’s PRs.

By adapting I’ve meant somthing like scanning the comments of each PR next to the tasklist info.
If the the tasklist is unfinished and/or the last two comments do not include +1, the would still be due to review, which could be represented graphically.

But as you said before, with 5-10 open PRs at a time the situation is not yet too confusing.

Yer, I think we pretty much solved this one.

1 Like

IF discourse were a QA system, one could mark this thread as solved. Or even promote the right answer.
I was just phantasizing about this lately. But this is completely OT.

You can a) close the topic as a mod and b) like single posts using the heart button.

1 Like

As this discussion has found a solution, I propose to a) close this thread or b) archive it.

I favour b) as of it’s more beautiful appearance in the thread list.

This may be helpful to some of us here: https://prs.paas.allizom.org/remotestorage/remotestorage.js

1 Like