Please review WINE chapter addition and linuxemu rename

Mon Jun 4 08:06:58 UTC 2012

Thanks all for the feedback, it'll take a while to get this incorporated
as I'm rather busy this week.

On Sat, 2012-06-02 at 00:46:03 +0200, Gabor Kovesdan wrote:
> As for the changes, I think you would get more review if you could post 
> a single patch. Having private repos is a very practical idea and I also 
> use it myself but people probably can easier take time for a review if 
> they don't have to clone and generate patches themselves. I can promise 
> one review if you send a patch.
> 
> Gabor

There's no need to clone. You can look at the diffs on the github page,
and if you have an account, you can also add comments to each line of
the commitdiff.

I find this easier than copy&pasting stuff into an email.

On Fri, 2012-06-01 at 20:05:09 -0600, Warren Block wrote:
> On Fri, 1 Jun 2012, Ulrich Spörlein wrote:
> 
> > in the process of adding the WINE section from the wiki to the handbook
> > (GCIN 2011 project, yeah I know, I was rather busy ...), I also took the
> > liberty of renaming it from linuxemu to compat. Also in the hope that
> > someday we might have a blurp on SVR4 compatibility, although I guess no
> > one is really interested in that.
> >
> > Anyway, please review the proposed changes at
> > https://github.com/uqs/freebsd-doc/commits/wine
> > or clone that branch and have a closer look.
> 
> Like Gabor suggested, a single diff makes it easier to see all that has 
> changed.

Except it's useless for the rename, as you'd need to diff the changes
after the rename against a different file. Something diff/patch cannot
do comfortably.

> 
> > Rendering of that branch can be found here:
> > https://www.spoerlein.net/handbook/compat.html
> 
> Just looking at the WINE section:
> 
>    "check the Wine AppDB on their main website."
> 
> "their main website" could be removed.
> 
> Those <programlistings> should be <screen> sections:
> 
> <screen>&prompt.root; <userinput>chroot /compat/i386</userinput>
> &prompt.root; /etc/rc.d/ldconfig start</screen>
> 
> And so on.  Note that the ending tag should go on the last line of 
> output.  If it is on a line of its own, there's an extra blank line in 
> the rendered version.
> 
> > Perhaps we need to keep the anchor names for the linuxemu sections, to
> > not break too many links out there. Is there a way to add aliases so we
> > could deprecated the linuxemu anchors over time?
> 
> I suspect it's all or nothing.  Leave them as-is, or change them and 
> anything that refers to them.  That may not be too hard; just do the 
> whole doc tree.  (Do translators translate ID names?  I don't know.)

On Sun, 2012-06-03 at 20:59:43 -0400, Benjamin Kaduk wrote:
> On Fri, 1 Jun 2012, Ulrich Spörlein wrote:
> 
> > Dear all,
> >
> > in the process of adding the WINE section from the wiki to the handbook
> > (GCIN 2011 project, yeah I know, I was rather busy ...), I also took the
> > liberty of renaming it from linuxemu to compat. Also in the hope that
> > someday we might have a blurp on SVR4 compatibility, although I guess no
> > one is really interested in that.
> >
> > Anyway, please review the proposed changes at
> > https://github.com/uqs/freebsd-doc/commits/wine
> > or clone that branch and have a closer look.
> 
> 2b3e8a6403909cdc752833e9c8b4f9196be9efe1 ("Move the linux section one 
> level down, add (empty) wine section.") seems to also include some 
> whitespace changes.

Yes, that's what the commit message says:

        Move the linux section one level down, add (empty) wine section.
        Fix some whitespace nits while here.

> In c89922e4d1e23acfe93eff9ae23cebbb9a93203a ("Add WINE section from 
> wiki.freebsd.org"):
> 
>          1353  +    such things. Hence, the contraction of "Wine Is Not an Emulator."
> 
> I do not think that 'contraction' is the best word here; I think that 
> 'acronym' might be better (as "acronym from").
> 
>    	1355 +    <para>Wine requires LDT support, but it is not available on RELENG_7 and
>    	1356 +    earlier versions. However, it works fine on versions 8.2 and 9.0 or newer.
> 
> You probably want to specify that this is versions of FreeBSD (i.e. &os;), 
> and maybe only use the word 'versions' once and &os; for later 
> appearances.
> 
>    	1359 +    comes quite cleanly from a "make install clean" command while in the wine
>    	1360 +    directory of ports; for how well individual applications work, check the
> 
> In addition to Glen's fix, I would just say "while in <filename 
> class="directory">ports/emulators/wine</filename>".
> 
>    	1362 +    website. Wine does not work out of the box on amd64 system. However, with a
> 
> Should be "systems" plural.
> 
> 
>    	1364 +    amd64 system (Diablo 2 works just fine). The following list is not
>    	1365 +    minimal, as I tried several different paths before I got one working.
> 
> We really don't like to use the first person in our documentation ("I"); 
> it would also be nice to attempt to turn the list into a minimal one 
> before finalizing the document.
> 
> Skipping the programlisting (please do heed Warren's comments),
> 
>    	1402 +    <para>For sound in Diablo 2 to work, run winecfg and set acceleration
> 
> winecfg should be in <command> markup.
> 
>    	1403 +    to "Emulation" (see Known Problems section above) then run "wine32
> 
> <quote> would probably be good here, too (I think this is what Glen meant 
> instead of "?).
> 
>    	1404 +    Game.exe." The problem seems to lie in the differences between OSS and
> 
> The command to run should be marked up using <userinput> and maybe in a 
> <screen> block, too.
> 
>    	1405 +    ALSA
> 
> Full stop at end of sentence.
> 
>    	1406 +    </para>
>    	1407 +    <para>3D acceleration is working with the 64bit nvidia driver provided
> 
> The document should be internally consistent about "32 bit" vs "64bit" vs 
> "64-bit" (I prefer the latter form, myself), but the first two appear in 
> this change.
> 
> I thought there was an &nvidia; entity that should be used here but I 
> can't find one; it is usually capitalized either nVidia or NVIDIA or 
> NVidia elsewhere in the tree.
> 
> This is x11/nvidia-driver, right?  That should probably be mentioned 
> explicitly.
> 
>    	1408 +    that you install the 32bit version (same version number) into the
> 
> This is the 32-bit version of nvidia-driver (as opposed to, say, wine), 
> right?
> 
> -Ben Kaduk
> 
>    	1409 +    chroot (tested with World of Warcraft, 8.0-RELEASE).

Thanks for the input, as I said, it might be easier to do the review on
github than to paste stuff into an email.

Expect an update by end of the week.

Cheers,
Uli