Some review feedback

[nicolo-ribaudo]

• two modules with the same source text but different URLs should probably not be equal according to ModuleSourcesEqual: we don't coalesce imports to two modules with the same URL just because they have the same text
• there is a typo in the new requirement of HostLoadImportedModule (necesasry)
  • separately, it would probably be cleaner to have this new requirement just as a new host hook, since we don't expect the host to be able to return a separate module record
• we cannot throw for import(randomObject)
• suggestion: the AO names are very hard to read, because we have both Modules and Module Sources, and given that they are AO names there are no spaces to disambiguate. What about calling sources just "Source" in all those AO? i.e. module.SourcesEqual(module2), module.GetSourceName(), GetSourceModuleRecord() (this last one is the currently worst named one)

[guybedford]

two modules with the same source text but different URLs should probably not be equal according to ModuleSourcesEqual: we don't coalesce imports to two modules with the same URL just because they have the same text

As discussed in the meeting today, this is the design of the method.

there is a typo in the new requirement of HostLoadImportedModule (necesasry)

Thanks, fixed in #36.

separately, it would probably be cleaner to have this new requirement just as a new host hook, since we don't expect the host to be able to return a separate module record

As discussed in the meeting today - this is not a requirement and it is possible (and expected to meet the host hook requirement) to coalesce into a different module record.

we cannot throw for import(randomObject)

Thanks, also fixed this in #36.

suggestion: the AO names are very hard to read, because we have both Modules and Module Sources, and given that they are AO names there are no spaces to disambiguate. What about calling sources just "Source" in all those AO? i.e. module.SourcesEqual(module2), module.GetSourceName(), GetSourceModuleRecord() (this last one is the currently worst named one)

As discussed in the meeting the term "ModuleSource" is supposed to clearly mean we are referring to the terminology used for a "Module Source Object". So seeing these two words together is the noun of operation for the methods. I think this terminology is consistently used, and allows us to clearly distinguish module source object operations. We could include some notes to clarify this disambiguation if necessary perhaps?