Re: [GIT PULL v4 00/21] libnd: non-volatile memory device support

[Dan Williams <dan.j.williams@intel.com>]

On Wed, May 27, 2015 at 5:42 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Thu, May 28, 2015 at 2:34 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>> On Wed, May 27, 2015 at 4:17 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>> On Thu, May 28, 2015 at 12:52 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>>>> On Wed, May 27, 2015 at 3:36 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>>> Hi,
>
> [...]
>
>>>>>>     2/ Update to latest NFIT UUID definitions (Toshi).  This
>>>>>>        merges cleanly with, and is identical to the include/acpi/
>>>>>>        NFIT enabling in Rafael's linux-pm.git/bleeding-edge branch.
>>>>>
>>>>> Well, I didn't expect you to send a pull request for this right away
>>>>> to be honest.
>>>>
>>>> No worries, we can address these concerns now...
>>>>
>>>>> Can you please pull from my acpica branch and rebase your patches on
>>>>> top of that by any chance?
>>>>
>>>> I noticed that bleeding-edge rebased from the last time I checked is
>>>> that branch stable enough to use as a baseline?
>>>
>>> There is a separate acpica branch (called "acpica") that's not going
>>> to be rebased.  Please use that one.
>>>
>>>>> And no, the "merges cleanly" part isn't sufficient as it'll create a
>>>>> mess of a history if merged together like that.  Can we do that
>>>>> properly instead?
>>>>
>>>> If I merge 'bleeding-edge' on top of v4.1-rc5 followed by this branch
>>>> and do a "git log include/acpi/acuuid.h" then the full history from
>>>> the 'bleeding-edge' branch shows up.
>>>>
>>>> I'm fine with doing the rebase, but I don't quite see the mess to
>>>> which you are referring.  Especially compared to the thrash of moving
>>>> our test baseline.
>>>
>>> People will not be running your test baseline, mind you.  They will be
>>> running the product of merging that with other stuff and for example
>>> the same change showing as two different commits in the history is not
>>> a particularly clean thing.
>>
>> That's what -rc kernels are for, to test your development baseline
>> against everything that came in during the merge window, i.e. when you
>> know you have a solid development baseline to reference.  Linus
>> reprimands late rebasing for good reason.
>>
>> Really, we're going to rebase 13,000 lines of new functionality and 20
>> patches to prevent recording some extra history around 200+ lines of
>> header definitions?  The history for those 200 lines being
>> autogenerated from another repo.  I struggle to resolve the risk
>> benefit tradeoff of going this route... are you sure this is a hard
>> gate for moving forward with this patch set?
>
> And how much time is it going to take to rebase it, actually?
>
> If all is so clean as you're suggesting, a "git rebase" should be
> sufficient for that really.  Is it not the case?

Of course the rebase is trivial, it's the testing that has gone into
the baseline being forfeited for no good reason that I take issue.

> I do believe that having a clean history in the repository is
> important, especially for big new and complicated features like this
> one.

Sure, in the general case, but this is one extra commit for
autogenerated acpica history.

> For the same reason I don't believe that rushing such features in no
> matter what is the right approach.
>
> If Jens decides to pull it regardless, it's his call, but I wouldn't
> do that if I were him.

I'm not going to push code around your objection.  I understand and
agree with the general policy, but in this specific case I believe an
exception is warranted.  If you still don't ack the approach I'll
proceed with the rebase.