[gmx-developers] Changes without reviews

Schulz, Roland roland.schulz at intel.com
Tue Feb 19 23:27:04 CET 2019


> Interesting discussion. An observation when looking at "My Open Issues"
> is that there are quite a few people who have way too many patches open
> simultaneously (including myself and other core-developers). This makes it
> difficult to keep them all up to date (as Roland noted, by rebasing) and many
> patches disappear from the front page and those that are on the front page
> have merge conflicts. A patch that is evidently not up-to-date to me signifies
> that the developer is not likely to work on it in the near future and hence
> there is little point to review the patch (if I have the competence to do so).
I agree that at the moment I also tend to not look at any patches which aren't up-to-date. But I don't think that's an ideal practice. There is really no good reason to have to keep them up to date just to get feedback. I don't know about others. But I have a couple of patches open which are marked as RFC (e.g. 8411 and 7505) but I never gotten a comment on. So there's nothing to work on until I get a comment. I don't know whether I should abandon them because no one sees value in them (and that's why no one has commented) or whether no one has noticed them.

And I'm skeptical that reducing the number of changes is a good solution. I like to create small changes to fix issues I notice while working on something larger or reviewing code. That tends to create multiple open small changes when I have time to work on GROMACS. It seems worse to try to combine multiple small changes into one larger or not immediately fixing issue when time is available because already to many other changes are open.

> >> However, I don't see that it will work with a combination of allowing
> >> anybody to submit anything, assigning reviewers automatically, and
> >> then saying that assignment implies it should be done within a week
> >> unless explicitly declined :-) That comes with the risk of spending a
> >> lot of time on minor cleanup changes, while we won't have time for
> >> the large things people are funded for.
I didn't suggest to automatically assign reviewers. And I hoped that people would ask for reviews respectfully so that those requests (and the ask to remove oneself if no intention of reviewing) wouldn't become distracting. If that isn't the case, we would need a mechanism to limit requests.

> >> There I think a better solution is to get better with the social
> >> planning at the bi-weekly telcos we have - but rather than merely
> >> stating what we are working on right now, we should get better at
> >> coming with suggestions ahead of time and then checking (a) if other
> >> people also want to prioritize that to help with reviewing, and (b)
> >> what other reviewing we can help with.

While I think that's a great suggestion, I don't see how this solves the issue of making gerrit more useable. Let's say Berk is planning to modernize some nbnxn component and I say I plan to help with reviewing that. How do I use gerrit to actually make sure I notice those changes? If we don't make changes to make the dashboard more useful how else do we do that?  

Roland


More information about the gromacs.org_gmx-developers mailing list