[m-rev.] Handling pull requests

[Julien Fischer]

Hi,

On Tue, Jan 8, 2013 at 12:13 PM, Paul Bone <paul at bone.id.au> wrote:
> On Mon, Jan 07, 2013 at 12:31:52PM +1100, Julien Fischer wrote:
>> On Mon, Jan 7, 2013 at 12:15 PM, Julien Fischer <jfischer at opturion.com> wrote:
>> > On Mon, Jan 7, 2013 at 12:09 PM, Peter Wang <novalazy at gmail.com> wrote:
>> >> On Mon, 7 Jan 2013 11:39:12 +1100, Julien Fischer <jfischer at opturion.com> wrote:
>> >>> On Mon, Jan 7, 2013 at 11:14 AM, Paul Bone <paul at bone.id.au> wrote:
>> >>>
>> >>> > I think the bit that needs discussion is how we do code reviews now that
>> >>> > we're using git and github.  What I'd like to acheive here is a way that any
>> >>> > of us can do code reviews, with any tools we like (eg vim and diff/patch or
>> >>> > github or git & vim).  And that the process should be simple regardless of
>> >>> > which tools we want to use.  Eg: I do not want to "force" github usage onto
>> >>> > anyone.
>> >>>
>> >>> The existing process, i.e. send the log message + patch to the mailing list,
>> >>> is fine IMO.
>> >>
>> >> Agreed.  That pull request emails don't contain the diff is a huge step
>> >> backwards.  Email + vim is better than any web interface.
>> >>
>> >> Non-core developers may prefer to send pull requests, but their changes
>> >> won't likely be very big anyway so not much to review.
>> >
>> > That's fine.  One of us can post the diffs corresponding to the pull
>> > request to the
>> > list.  (Most of the pull requests I've seen so far certainly require
>> > some discussion.)
>>
>> For instance, Michael Richter's README.bootstrap change, that Paul
>> just committed
>> is wrong in number of respects and *should* have been reviewed before
>> it was commited.
>> (For example, configuring the bootstrap compiler with
>> --enable-libgrades=asm_fast.gc
>> will result in a broken installation on Mac OS X; the correct option
>> to use would be
>> --disable-most-grades.)
>
> (I've CC'd Micheal as he's not currently on the lists, and should be.)
>
> I've never been sure about what --disable-most-grades means.  Sure it
> "Disables most grades" but what I want to know is which ones are kept.

When in doubt, read the documentation.  From the ``Fine Tuning'' section
of INSTALL:

#
#          The option --disable-most-grades reduces the set of installed grades
#          to a "minimum" level for developers (just the default grade and the
#          grades corresponding to the `--debug' and `--high-level-code'
#          options).

...

> This is not what I want configure to do when I only want enough Mercury to
> bootstrap with, specifically:
>     .debug is redundant as .decldebug is present.
>     I don't need debugging at all if I'm bootstrapping.
>     I only need one of (the best of) the LLC or HLC grades.

It's the closest thing we currently have that can be used portably.

> Maybe we should have an option such as --enable-one-grade-only

I agree.  I prefer --enable-minimal-install as the name though.  (It fits
in better with the existing --enable-minimal-hlc-trail-install option.)

If there's general agreement as to what the name should be I'll go ahead
and implement it.

As and aside: why is INSTALL.git named that, but referred to as INSTALL_GIT
inside the file?

Cheers,
Julien.