[edk2-devel] [edk2] [PATCH] OvmfPkg: QemuVideoDxe: Int10h stub for Windows 2008 R2 SP1 (stdvga, QXL)

[David Woodhouse]

On Thu, 2019-06-20 at 16:41 +0200, Laszlo Ersek wrote:
> We've never merged pull requests before, but I remember that you prefer
> those (because you dislike an edk2 subsys maintainer rebasing your
> branch from your original fork-off point). So I'm not asking for the
> patch email because I insist on using "git-am". I'm asking for it
> because we'd like to review every patch on the list, for now.

Going back to this... I generally prefer *everyone* to use the git
workflow as $DEITY intended it, not just for my own submissions.

When a developer writes and — perhaps more to the point — tests a
patch, they have done so against a specific version of the project.

With the proper git workflow, their contribution is merged with the
correct parent information. The version control system is doing its job
correctly and recording what actually happened; not an approximation of
it.

With git-am being used to apply those contributions on a later version
of the tree, there is a *theoretical* possibility that some other
seemingly innocuous change elsewhere in the tree just happens to break
the newly-submitted code.

The thing is: over time, at scale, the chances of that theoretical,
highly unlikely, incompatibility happening approach 1.

Anyone who has experience maintaining out-of-tree patches for any
project over time will have seen this. Your patchset suddenly doesn't
work, although it applied cleanly to the source tree, and you have to
go hunting for whatever strange locking or other environment issue, or
API semantics, broke it since you last updated.

If — no, WHEN — this kind of thing happens, you can end up with code
being merged which, according to the recorded history, never actually
worked at all. There is no way to use tools like 'git bisect' to find
the working point, and what went wrong. It simply *never* worked,
because the true history has been thrown away and not stored in the
version control system. The version control system had *one* job, and
you haven't allowed it to do that.

And yes, it's unlikely and infrequent. But it *does* happen, and that's
why we should use the tools properly as a matter of habit.

For my own patches of course I do a 'git pull --rebase' occasionally to
ensure that it all still works when my patches are applied on top of
the git master branch. But even with the best intentions, that isn't
really good enough. Because all the exhaustive manual testing of
various corner cases (adding multiple boot targets, checking that
selecting each of them does the right thing, turning 64-bit BARs on and
off and doing builds with various different configs) is not happening
when I do that 'git pull --rebase' smoke test. So even with me doing
some basic retesting, it's possible that something ends up getting
committed that, for some esoteric corner case, apparently never worked
even though it *did* actually work when I wrote the code. And when we
realise that in a year's time and look back, the version control system
won't help us because we didn't use it properly.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#42747): https://edk2.groups.io/g/devel/message/42747
Mute This Topic: https://groups.io/mt/32093442/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5174 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20190624/7cbb5edf/attachment.bin>