[MPlayer-dev-eng] [PATCH] Solaris support for SSE(2) autodetection

[The Wanderer]

Milan Jurik wrote:

> Hi,
> 
> I never contributed here yet, so if I'm doing something wrong, then
> I'm sorry. Here is small patch for cpudetect.c, to support SSE/SSE2
> autodetection:
> 
> 
> --- mplayer/cpudetect.c Sun Sep 14 11:31:53 2008
> +++ mplayer-fixed/cpudetect.c   Sun Sep 14 11:43:17 2008

In procedural terms, you're doing three things wrong which I see - but
they all seem to be minor enough to not need a resubmitted patch.

First, you made the patch from outside the source tree rather than from
the root of the source tree. As a result, the patch would have to be
applied with '-p1' rather than the preferred '-p0'.

Second, you made the patch against a second local copy rather than
against the SVN repository - i.e., you used 'diff' rather than 'svn
diff'. Avoiding this would have also avoided the first problem.

Third, you posted the patch inline with the mail, rather than as an
attached file. In many cases this will lead to mangled and invalid patch
files, which need hand fixing or are outright unusable. In this
particular case it seems at a glance to have come through OK.

For future reference, read DOCS/tech/patches.txt for the (mostly) full
patch submission rules.


Of course, all of that is totally ignoring the contents of the patch, on
which I am not presently qualified to pass judgment.

-- 
       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

Secrecy is the beginning of tyranny.