On Mon, Oct 04, 2010 at 12:03:22PM -0400, Johan Tibell wrote:
I argue that's the reason we don't see many small, but important, changes to the core libraries.
FWIW, I graphed the number of patches per year in unix and base here: http://lambda.haskell.org/~igloo/lib_patches.png (Library_submissions started in Oct 2006) but I'm not sure it really tells us much, as it doesn't say how many patches were "maintenance" rather than "development".
Non-functional changes should be allowed without review by the libraries list.
As Ross said, simple, obvious improvements are applied without going through the process. A common example is patches that just add haddock docs to previously doc-less functions. Note that some patches are obvious when being created, but are hard to sanity check afterwards. For example, when making a patch that just moves functions around, it's easy to be confident it's safe when creating the patch, but when someone else sees it it just looks like a load of code being added and removed. It's even worse if other changes are made in the same patch. In general, it makes life easier for the committers if the libraries process is used, so it is not down to us to review the patches.
or attached to a "please merge" ticket on the bug tracker.
That's not what 'merge' is for. I improved the description a couple of weeks ago, so it's hopefully clearer now: fixed in HEAD; please merge to STABLE Next status will be 'merge' The right state is 'patch': please review Next status will be 'patch'
Only changes that don't affect the API or performance (in non-trivial ways) should be allowed in without a review.
But you need some amount of review in order to know what a patch does. For example, it was only when reading a recent patch Sun Aug 29 13:02:45 BST 2010 * Performance improvements to Data.Map that I discovered that it added DEPRECATED pragmas for two functions.
It's possible that once a commit that should have been reviewed gets commited. I suggest we adopt an "ask for forgiveness, not for permission" policy in these cases. We can always roll back changes.
IIRC, part of the motivation for the library process was that changes were being made in the HEAD, getting into release, and then people discovered that they caused problems.
someone (e.g. Ian)
You mentioned my name a few times; I'm not too keen on proposals that mean more work for me!
My suggestion is that a contributor without commit access simply creates a ticket and assigns it to someone with commit access (e.g. Ian).
If you do have a patch that doesn't need to go through the libraries process, the best thing to do is to file a ticket and set its status to 'patch'. Assigning it to me is unlikely to get it applied faster, but means no-one else is likely to look at it.
Aside: Asking who has commit access in an email to the libraries list will quickly get you a link to the libraries process (I tried!).
Well, if you want to get a patch committed, then that's normally what you need, so that seems like a reasonable response to me.
I assume that contributor test their patches, just as they would test them before submitting them to a package not maintained by the libraries list.
FWIW, it's quite common for submitted patches to break the build (e.g. platform-specific breakage, or new code has warnings, or even just silly mistakes that couldn't possibly compile (e.g. there was one in the past week which added an unmatched "endif" to a makefile)). Thanks Ian