Danek Duvall wrote:
> > http://cr.opensolaris.org/~talley/6667461_6667462_6667464.inc.1.2/
> >
> > > > > mutt/Makefile.sfw:
> > > > >
> > > > > o line 129: This should probably be /etc/mutt.
> > > >
> > > > I wavered a bit on this, but ultimately chose
> > > > /usr/share/mutt/etc in part to avoid conflicts with the
> > > > system's /etc/mime.types. But I can be persuaded here.
> > >
> > > No need to put it in /etc directly; /etc/mutt would be
> > > appropriate. It should go under /etc because /usr is not always
> > > mounted read-write, so it's inappropriate to put possibly
> > > editable configuration files there.
> >
> > Okay, agreed.
>
> Except that I'm not sure we should be shipping any configuration
> files that are loaded by default. Either for mutt or for slang. I
> think it's fine to have sample rc files in /usr/share/wherever,
> which can then be copied into /etc, but I don't see the point in
> shipping some random config by default.
The configuration in /etc isn't random -- it is a small set of sane
defaults that the mutt developers have determined to be generally
useful. I personally do rely on this configuration. Forcing users to
duplicate this default configuration in their own .muttrc seems unduly
burdensome. It would also be inconsistent with other OSs that ship
mutt.
> ...
>
> Further comments on the review:
>
> fetchmail/Makefile.sfw:
>
> o line 70: I see from the package prototype that you're making
> /usr/bin/fetchmailconf a symlink to the .py file in
> vendor-packages. You should construct the proto area to match
> the package.
Done.
> Is fetchmailconf.py ever imported by anything, or is it intended
> only to be executed? If the latter, it shouldn't be in
> vendor-packages at all.
It is not imported by anything else in the package. I do not know
whether the intent of the developer was to make it available for
import elsewhere, so leaving it in vendor-packages and linking to it
seems like the safest choice. If you feel strongly about this, I
can install it directly in usr/bin.
> SUNWfetchmail/Makefile:
>
> o line 30: If you're going to start pulling prototype_{i386,sparc}
> from common_files, then you should probably do this for the rest
> of the packages, too. If that doesn't appeal, do it the way
> everyone else does.
Done.
> SUNWmuttr/Makefile:
>
> o line 36: Please just keep a separate copy in this directory (if
> you really want a SUNWmutt, anyway).
Okay, done.
> SUNWmuttr/prototype_com:
>
> o line 41: You made a change, but didn't make it everywhere. This
> and the Makefile say renamenew, but the actual files and the
> pkginfo say renameold. Pick one.
Good catch. Fixed.
> tools/Makefile:
>
> o Why is this new?
Because this is an incremental webrev and that file wasn't initially
edited. The new webrev (below) shows the changes.
> tools/sunman.sh:
>
> o line 33: No need to put $PATH in $PATH.
Removed.
> o line 66: How many platforms is this script goin to run on?
Removed.
> o line 102: "Amend".
You are quite thorough. Fixed.
Newest webrev is at:
http://cr.opensolaris.org/~talley/6667461_6667462_6667464.3
Steve
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 185 bytes
Desc: not available
URL:
<http://mail.opensolaris.org/pipermail/sfwnv-discuss/attachments/20080310/31e43e4d/attachment.bin>
