tech-pkg archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: wip/mk/ cleanups

On 2014-05-06 08:30, Thomas Klausner wrote:
> Hi Richard!
> Thanks for all your work on this! The patches look quite good.

Are you OK with the WRKSRC symlink?  (for now anyway)

> I have one question and a stylistic comment.
> On Mon, May 05, 2014 at 07:54:17PM -0400, Richard Hansen wrote:
>>   * create NO_CVS_CACHE, NO_GIT_CACHE, and NO_HG_CACHE for consistency
>>     with NO_SVN_CACHE (so users can easily disable caching)
>>   * create NO_{CVS,GIT,HG,SVN}_CACHE.${id} so that packages can
>>     disable caching
> We usually avoid negative variable names, if possible, since e.g.
> NO_CVS_CACHE=no is a bit hard to understand. Can you make these
> positive? Perhaps CREATE_HG_CACHE or CACHE_HG or something similar?

I actually added positively named variables to cvs-, git-, and before I noticed that already had
NO_SVN_CACHE.  Are you OK with breaking compatibility by renaming
NO_SVN_CACHE?  If so, I'd be happy to rename everything to
CACHE_{CVS,GIT,HG,SVN} (I prefer positive variable names too).

Alternatively I can support both CACHE_SVN and NO_SVN_CACHE and add a
deprecation warning if defined(NO_SVN_CACHE).

>> Subject: [PATCH 05/13] use :Q modifier to escape special shell characters
> Did this actually fix anything or is it just for safety?

Just for safety.  Sorry for not mentioning that in the commit message;
I'll update it in the next re-roll.


Home | Main Index | Thread Index | Old Index