LGTM!

On Mar 17, 2014, at 3:30 PM, Ben Langmuir <[email protected]> wrote:

> Updated patch attached that switches the preferred spelling to 
> ‘module.modulemap’ and also updates docs/Modules.rst.
> 
> Ben
> 
> <modulemap.patch>
> 
> 
> On Mar 17, 2014, at 2:23 PM, Ben Langmuir <[email protected]> wrote:
> 
>> 
>> On Mar 17, 2014, at 1:44 PM, Richard Smith <[email protected]> wrote:
>> 
>>> I don't think the extra stat calls are a meaningful cost -- most of the 
>>> time, none of the files will be present, so we'll perform all four (failed) 
>>> stats, so if they *are* significant, we have bigger problems…
>> 
>> Good point.
>> 
>>> 
>>> The old/new default only affects cases where both files are provided (which 
>>> is a clear sign that the project is trying to be compatible with both the 
>>> old and the new form). In such cases, it seems to me that we should pick 
>>> the newer file -- that way, when we come to turn off support for the old 
>>> names, we won't introduce a change in behavior.
>> 
>> Okay I buy this.  I’ll prefer the new spelling.
>> 
>>> 
>>> I'm not overly concerned about the warning. I think most affected people 
>>> would be Xcode Clang users, so you guys should do whatever works best for 
>>> you to transition your existing modules users.
>> 
>> Hmm, after changing to prefer the new spelling, I’m not sure this is 
>> interesting enough anymore.  My primary concern was that we might 
>> accidentally hide errors in module.modulemap files by having stray 
>> module.map files around.
>> 
>> Thanks,
>> 
>> Ben
>> 
>>> 
>>> On Mon, Mar 17, 2014 at 1:26 PM, Ben Langmuir <[email protected]> wrote:
>>> That is the direction I would like to go eventually, but I didn't want to 
>>> deprecate the old spelling in the short term, since we still have lots of 
>>> module.map files that would need to be moved. The advantage of checking for 
>>> the old name first is that we save stat calls by looking for the more 
>>> likely to hit name first.
>>> 
>>> Ben
>>> 
>>> On Mar 17, 2014, at 11:56 AM, Richard Smith <[email protected]> wrote:
>>> 
>>>> I have no objection to the name change.
>>>> 
>>>> It seems that the intent is to move away from the old module.map name 
>>>> entirely. That being the case, should we look for the new name first, and 
>>>> consider adding a warning if we find the old name?
>>>> 
>>>> On Mon, Mar 17, 2014 at 11:33 AM, Argyrios Kyrtzidis <[email protected]> 
>>>> wrote:
>>>> + Richard.
>>>> 
>>>> Do you have an objection to what Ben proposes here ? The “.map” extension 
>>>> is unfortunate and we’d really like to move “.modulemap”; the best chance 
>>>> to change it is now, before first class support for user modules enters 
>>>> the picture.
>>>> 
>>>> On Mar 17, 2014, at 10:40 AM, Ben Langmuir <[email protected]> wrote:
>>>> 
>>>> >    This name, while more verbose, plays more nicely with tools that use
>>>> >    file extensions to determine file types. The existing spelling
>>>> >    'module.map' will continue to work, and at least for now will take
>>>> >    precedence if both files exist within a directory. In a future patch, 
>>>> > I
>>>> >    intend to add a warning for when both files could be found.
>>>> >
>>>> >    In frameworks, this new filename will only go in a new 'Modules'
>>>> >    sub-directory.
>>>> >
>>>> >    Similarly, add a module.private.modulemap corresponding to
>>>> >    module_private.map.
>>>> >
>>>> > <modulemap.patch>
>>>> 
>>>> 
>>> 
>>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> [email protected]
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to