The libraries proposal process and containers

[Thomas Schilling]

Hi Milan,

as you admitted, some of the commits were overly hasted, but everyone
makes mistakes, so don't worry.  You'll know next time.   The tickets
for which the discussion period was over were completely fine.  You
don't have to ask Ian or any of the Simons if there were no issues
raised and you have properly validated.

For #4311, #4312, #4333, you admitted skipping the proper process, but
I think we don't need to revert anything.  Minor changes or
non-API-visible changes (like adding a benchmark suite) are fine, too.

The only real issue I think is the INLINE vs. INLINABLE change which I
didn't feel was resolved.  There was some disagreement about whether
the bloat is worth it (since INLINABLE didn't not improve
performance).  The discussion seemed to move towards the compromise
you just committed (INLINABLE for now, think about INLINE features for
after 7.0.1), but it wasn't clear that this was the agreed comprise.

In the future:  If there is no clear consensus just send an email
summarising the discussion (as you understand it) and state what
appears to be the consensus (or propose one).  This will hopefully
focus the discussion and find a clear consensus.

This is similar to the process we have in place for platform proposals
and I think it's good one.

Anyway, we make mistakes -- we learn from them.

Keep up the good work!

/ Thomas

On 24 September 2010 18:33, Milan Straka <fox at ucw.cz> wrote:
> Hi,
>
> to clatify what I pushed:
>
> - 4277 with discussion period over (dons)
> - 4279 with discussion period over (dons)
> - 4280 with discussion period over (tibbe)
> - 4311 without explicit period (my mistake), there were some reactions,
>  I created v2 of the patch
> - 4312 without explicit period (my mistake), there were some reactions,
>  I created v2 of the patch
> - 4333 without explicit period (my mistake)
>  This is just consistency with 4277, 4278 and 4280.
> - INLINE => INLINABLE issue which was being sorted out at libraries@
> - some trivial changes like warnings and rearranging OPTIONS and LANGUAGE.
>
> All these things do not touch API. The performance patches have
> benchmark results on them.
>
> I pushed hastily as I wanted to sort out the INLINE => INLINABLE issue
> and I was working on the top of my repo with all these performance
> changes.
>
> I will not do it again.
>
> Sorry,
> Milan
>
>> Hi,
>>
>> I am terribly sorry if I did it wrong.
>>
>> I posted the tickets to libraries, there were some discussion about some
>> of them.
>>
>> After sorting the INLINE => INLINABLE issue (I agree that in a hurry)
>> I pushed all the tickets (I was working on top of my repo).
>>
>> Once again, if I should have waited more, sorry. Rollback if
>> I overstepped my authority.
>>
>> Sorry,
>> Milan
>>
>> >
>> > Milan just pushed all the following patches directly to the containers
>> > repo, and closed all the tickets associated with the effort.
>> >
>> >       * Fix warnings in Data.Map and Data.Set.
>> >       * Finish the started worker/wrapper transformation.
>> >       * Merge all the OPTIONS and LANGUAGE module pragmas.
>> >       * Remove most INLINE from Map, Set, IntMap and IntSet.
>> >       * Comment tests and benchmarks on foldlWithKey'
>> >       * Worker/wrapper transformation for Data.IntSet.
>> >       * Compile only the benchmark source, not the Data/*.hs.
>> >       * Add criterion-based benchmark for IntSet.hs
>> >       * Add a testsuite for Data.IntSet.
>> >       * Further improve Data.Set balance function
>> >       * Further improve Data.Map balance function
>> >       * Changing delta to 3 in Data.Set.
>> >       * Changing delta to 3 in Data.Map.
>> >       * Correct Data.Set Arbitrary instance never to return unbalanced trees.
>> >       * Correct Data.Map Arbitrary instance never to return unbalanced trees.
>> >       * Improve Data.Set benchmark.
>> >       * Improve benchmark infrastructure and Data.Map benchmark
>> >       * Improve the performance of Data.Set balance function
>> >       * Improve the performance of Data.Map balance function.
>> >       * Improve performance of Data.Set union and difference operations
>> >       * Improve performance of Data.Map union* and difference operations
>> >       * Make the Set store the elements evaluated (bang added).
>> >
>> > Have *any* of these patches been proposed for review? containers is a
>> > critical library, and under libraries@ maintainance. This is not what
>> > you do prior to a release.
>> >
>> > Ian, Simon M.: did you get a chance to sign off on this?
>> >
>> > -- Don
>> >
>> > _______________________________________________
>> > Libraries mailing list
>> > Libraries at haskell.org
>> > http://www.haskell.org/mailman/listinfo/libraries
>> >
>> _______________________________________________
>> Libraries mailing list
>> Libraries at haskell.org
>> http://www.haskell.org/mailman/listinfo/libraries
>>
> _______________________________________________
> Libraries mailing list
> Libraries at haskell.org
> http://www.haskell.org/mailman/listinfo/libraries
>



-- 
Push the envelope. Watch it bend.