in reply to pmdev: patches to consider (feature idea)

Since approving a patch allows code to run in the privileged environment, it is the power to do anything gods can do and so if we want to allow "vote to apply" then we'd need a new group that lies between gods and pmdev in power and thus selectivity. But I think that isn't the best solution.

I think the solution is better communication on several fronts.

Ways to encourage more discussion of patches would be good. We could just use note to reduce the effort required for this and to allow all members to see these discussions. I've already been wanting to allow all members to see the list of patches and their descriptions (but not the code details, unfortunately, since the security level of the code is still not high enough for my comfort).

Those writing patches need to make more effort to test their patches. I've had more than one person get impatient to get a patch applied only find out that their patch has bugs that I found without applying the patch (which means the original author could have found the bug with the same effort). At the very least, take your patch and prepend "use strict; my( $q, $query, $USER, ... );" to the front of it and verify that it compiles. Do this no matter how small the patch is.

For most patches, dummy up enough functions and variables so that you can see the resulting HTML (or whatever it produces). It isn't that much work. I've done it many times.

Other pmdevils commenting on patches (including "code review") will help with the lack of feedback and with getting comfortable enough with a patch to apply it.

I think there is also sometimes a problem with expectations. Applying patches takes weeks not hours. Sure, sometimes I appear to write and apply a patch myself in a matter of hours, but that is usually preceeded by weeks of getting a feel for the "policy" implications. If someone else writes a patch, then I usually haven't had the time to think about the wider implications.

Also, schedules don't line up so it can take a week or more to get a chunk of time appropriate for the consideration and application of a patch.

Believe me, I've got lots of patches that have been started many months ago that haven't been applied. I'm afraid that is the nature of changes to PM until several fundamental changes happen that are of big enough scope that I don't plan to be personally involved in them.

I recently went through all of the old patches and applied a few that had been sitting around and found that the rest that I'd had on my internal to-do list had either been applied by other gods or had been superceeded by newer patches. So I don't think there are any patches more than a few weeks old that are waiting to be applied.

I think we also need a "please don't apply" convention so it is trivial to distinguish old, unapplied patches that should be considered and old, unapplied patches that have been rejected. A reply noting the reject is helpful, but we recently had a patch applied that was noted in PMDiscuss and in the pmdev wiki as being unacceptable but it still got applied. So I think a direct indicator is worthwhile. Perhaps put "***Don't apply" plus a comment as to why at the start of the patch code and a "!" at the start of the patch description?

I think part of the problem is that it is hard to wait if you don't think you'll be notified when something happens. It'd be nice if, after applying a patch, there was a field for /msg'ing the patch author to encourage feedback.

Having a "*" show up on the "Patch Lister" link in the PmDev Nodelet if there are patches one hasn't seen yet would help.

                - tye
  • Comment on Re: pmdev: patches to consider (no, thanks)