Library Export refactor, phase 2 #2424
Conversation
| @@ -8,12 +8,15 @@ namespace Microsoft.Dnx.Compilation | |||
| /// </summary> | |||
| public interface ILibraryExporter | |||
| { | |||
| LibraryExport GetLibraryExport(string name); | |||
There was a problem hiding this comment.
Prefer the old names. The exporter is providing exports, not actually exporting anything as a verb.
There was a problem hiding this comment.
fixed, but just called it GetExport and GetAllExports
analogrelay
force-pushed
the
anurse/2226-refactor-export-2
branch
2 times, most recently
from
b59161e
to
3aa8fe6
Compare
| _graph = _libraryInfoThunk().ToDictionary(ld => ld.Name, | ||
| StringComparer.Ordinal); | ||
| var libraries = _librariesThunk(); | ||
| _graph = libraries.ToDictionary(l => l.Identity.Name, StringComparer.Ordinal); |
There was a problem hiding this comment.
Why do we need so many dictionaries? We're going to need to do a serious review of all of our allocations because this looks like overkill.
There was a problem hiding this comment.
Actually, I think we can combine these into a single dictionary storing a tuple (or a custom class). We need to be able to retrieve both the externally-used Library class and the internal LibraryDescription. I can merge the two into one dictionary though, either by putting Library on LibraryDescription or creating a tuple.
analogrelay
force-pushed
the
anurse/2226-refactor-export-2
branch
from
3cc9c67
to
02c9a98
Compare
analogrelay
force-pushed
the
anurse/2226-refactor-export-2
branch
from
02c9a98
to
c0cebb0
Compare
It's not quite finished yet (there's a Stack Overflow error in dnu build, yaaay!), but I wanted to open the review before I head off for my brief holiday.
/cc @davidfowl