andreasohlund / APIComparer

Compares NuGetPackages/Assemblies and displays changes in the public api.
MIT License
51 stars 4 forks source link

Teach ComparerEngine to not choke on native binaries #48

Closed nulltoken closed 9 years ago

nulltoken commented 9 years ago

Fix #47

Some additional proposals have been added. Feel free to tell me if you'd prefer me to drop them. I'll be happy to comply :wink:

nulltoken commented 9 years ago

Still a WIP. I'll work on the fix per se tomorrow

nulltoken commented 9 years ago

@andreasohlund @SimonCropp No longer a WIP, ready for review.

nulltoken commented 9 years ago

A different approach would be to actually move this IsManagedAssembly() call in NuGetDownloader.DownloadAndExtractVersion(). Would you prefer this one?

andreasohlund commented 9 years ago

Since we also support , use the .exe, comparing local files as well I think the current solution is the best one

nulltoken commented 9 years ago

@andreasohlund Duh. I didn't notice your comment earlier and just pushed a fixup to showcase the alternative approach. Still preferring the first one?

nulltoken commented 9 years ago

I'm going to rollback the fixup as indeed this wouldn't help wouldn't we rely on the NuGetDownloader types. BTW, why do we have two of them?

andreasohlund commented 9 years ago

Yea, I prefer the first version

Regarding the duplication, I think they will diverge soon so we kept them separate for now

Copy paste reuse ftw!;)

On 05 Sep 2015, at 14:43, nulltoken notifications@github.com wrote:

I'm going to rollback the fixup as indeed this wouldn't help wouldn't we rely on the NuGetDownloader types. BTW, why do we have two of them?

— Reply to this email directly or view it on GitHub.