On Mon, 7 Apr 2008, Attila Lendvai wrote:
On Mon, 7 Apr 2008, Daniel Herring wrote:
I'd prefer if you held off on this one.
In particular, trivial-features bothers me. Libraries probably shouldn't modify the features set by an implementation. I would feel much more comfortable if all names were prefixed to reduce conflict. e.g. set :tf-darwin and :tf-linux instead of :darwin and :linux
fwiw, i don't see in what way would it be better if the otherwise obvious symbol names were prefixed. it would just make the user code less readable. if tf modifies *features* in a wrong way then it needs to be fixed.
Until we get a CDR/other generally accepted spec on how *features* should reflect the platform, there are no wrong settings in *features*. Since this is a global value, special care must be taken to not break code which has already been tuned for an implementation. It seems rude for a nested library to change common global settings.
Putting a prefix on all customized features - minimizes conflicts with implementation-specific flags - is explicit about intent; #+:unix says "I want unix". #+:tf-unix says "I want a tf-compliant unix".
Since we probably can't alter the spec, a prefix may be reasonable.
Is there a structural reason to merge grovel with cffi? If not, could they stay separate? Does that reduce the core CFFI dependencies? You probably integrated grovel for trivial-features... Could tf search for :cffi-grovel in *features* and implement a fallback if not found?
i'm not following you on this... cffi-grovel is a standalone .asd already, it's just practical to keep it in the same repo as cffi itself because it tightly depends on cffi and it's rather small to be a separate project.
This branch adds three new dependencies. Babel for string translation is understandable. Alexandria and trivial-features are somewhat less obvious. Without reading the darcs branch, I was trying to ask whether these dependencies are only due to the integration of cffi-grovel, or whether they were being used in the core cffi functions as well.
If the core cffi is introducing these dependencies, then I'm checking that we gain something tangible in return. If these two are just for cffi-grovel, but cffi can be used without grovel, then sure; add grovel to cffi, but please make it clear in the install docs which deps are needed for what. Something like "Babel is needed for CFFI; Alexandria and trivial-features are only used by cffi-grovel."
I don't want to hold things back (cue CFFI-0.9.2 vs debian-stable jokes); but I would like to avoid unnecessary dependencies.
Later, Daniel